Uploaded image for project: 'Hive'
  1. Hive
  2. HIVE-1497

support COMMENT clause on CREATE INDEX, and add new command for SHOW INDEXES

Details

    • Improvement
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 0.7.0
    • 0.7.0
    • Indexing
    • None
    • Reviewed

    Description

      We need to work out the syntax for SHOW/DESCRIBE, taking partitioning into account.

      Attachments

        1. HIVE-1497.4.patch
          52 kB
          Russell Melick
        2. HIVE-1497.5.patch
          49 kB
          Russell Melick
        3. HIVE-1497.6.patch
          58 kB
          Russell Melick
        4. HIVE-1497.7.patch
          58 kB
          Russell Melick
        5. HIVE-1497.8.patch
          58 kB
          Russell Melick
        6. hive-1497.p1.patch
          9 kB
          Russell Melick
        7. hive-1497.p2.patch
          18 kB
          Russell Melick
        8. hive-1497.p3.patch
          26 kB
          Russell Melick

        Issue Links

          Activity

            jvs John Sichi added a comment -

            Followup for HIVE-417.

            jvs John Sichi added a comment - Followup for HIVE-417 .
            jvs John Sichi added a comment -

            Note recent improvements on DESCRIBE TABLE in HIVE-558.

            For command syntax, Hive usually mimics MySQL, so I guess just SHOW is good enough (no DESCRIBE needed):

            http://dev.mysql.com/doc/refman/5.0/en/show-index.html

            jvs John Sichi added a comment - Note recent improvements on DESCRIBE TABLE in HIVE-558 . For command syntax, Hive usually mimics MySQL, so I guess just SHOW is good enough (no DESCRIBE needed): http://dev.mysql.com/doc/refman/5.0/en/show-index.html
            namit Namit Jain added a comment -

            btw, HIVE-558 just got committed.

            namit Namit Jain added a comment - btw, HIVE-558 just got committed.
            jvs John Sichi added a comment -

            For the implementation, it should be possible to follow the pattern for the existing SHOW PARTITIONS command.

            • showStatement in Hive.g
            • DDLSemanticAnalyzer.analyzeShowPartitions
            • ShowPartitionsDesc
            • DDLTask.showPartitions; for returning multiple fields per output row, use out.write(separator)
            • Hive.java does not currently have a getIndexes method, so you'll need to add that. But IMetaStoreClient does have a listIndexes method already, so you can call that.
            jvs John Sichi added a comment - For the implementation, it should be possible to follow the pattern for the existing SHOW PARTITIONS command. showStatement in Hive.g DDLSemanticAnalyzer.analyzeShowPartitions ShowPartitionsDesc DDLTask.showPartitions; for returning multiple fields per output row, use out.write(separator) Hive.java does not currently have a getIndexes method, so you'll need to add that. But IMetaStoreClient does have a listIndexes method already, so you can call that.
            rmelick Russell Melick added a comment -

            Preliminary patch that seems to work for showing indexes. No unit tests, and doesn't deal with combination of indexes and partitions.

            rmelick Russell Melick added a comment - Preliminary patch that seems to work for showing indexes. No unit tests, and doesn't deal with combination of indexes and partitions.
            rmelick Russell Melick added a comment -

            The wiki will need updating once this is committed.

            http://wiki.apache.org/hadoop/Hive/LanguageManual/DDL

            rmelick Russell Melick added a comment - The wiki will need updating once this is committed. http://wiki.apache.org/hadoop/Hive/LanguageManual/DDL
            rmelick Russell Melick added a comment -

            Previous patch was missing some changes due to a bad diff.

            rmelick Russell Melick added a comment - Previous patch was missing some changes due to a bad diff.
            rmelick Russell Melick added a comment -

            Coding complete, but unit tests not added yet. Does not deal with partitions either.

            rmelick Russell Melick added a comment - Coding complete, but unit tests not added yet. Does not deal with partitions either.
            jvs John Sichi added a comment -

            As discussed in last week's conf call, we'll need a followup for a separate DESCRIBE command (which will cover properties and partitions).

            jvs John Sichi added a comment - As discussed in last week's conf call, we'll need a followup for a separate DESCRIBE command (which will cover properties and partitions).
            rmelick Russell Melick added a comment -

            Modify SHOW INDEX grammer to use ON keyword instead of FROM/IN to align with other index commands.

            rmelick Russell Melick added a comment - Modify SHOW INDEX grammer to use ON keyword instead of FROM/IN to align with other index commands.
            rmelick Russell Melick added a comment -

            Should also update the Index wiki:

            http://wiki.apache.org/hadoop/Hive/IndexDev

            rmelick Russell Melick added a comment - Should also update the Index wiki: http://wiki.apache.org/hadoop/Hive/IndexDev
            he yongqiang He Yongqiang added a comment -

            + /*if (!tbl.isIndexed())

            { + console.printError("Table " + tableName + " does not have any indexes"); + return 1; + }

            */

            need to be removed.

            setFetchTask(createFetchTask(showIndexesDesc.getSchema()));
            

            need to be removed.

            There is no index comment in the new testcase. Can you add some test cases?

            SHOW INDEX ON shidx_t1; right now lists all indexes on the given table? Can we also support show one index? i mean sth like : 'show index index_name on tbl' (you can do it in a follow up jira)

            Right now the show index output format is:

            +idx_name            	tab_name            	col_name            	idx_tab_name        	idx_type            	comment             
            +	 	 	 	 	 
            +	 	 	 	 	 
            +idx_1               	show_idx_full       	key                 	default__show_idx_full_idx_1__	compact             	
            +idx_2               	show_idx_full       	value1              	default__show_idx_full_idx_2__	compact             	
            +idx_3               	show_idx_full       	value2              	default__show_idx_full_idx_3__	compact          
            

            John, is this intentional?

            Otherwise, the patch looks good.

            he yongqiang He Yongqiang added a comment - + /*if (!tbl.isIndexed()) { + console.printError("Table " + tableName + " does not have any indexes"); + return 1; + } */ need to be removed. setFetchTask(createFetchTask(showIndexesDesc.getSchema())); need to be removed. There is no index comment in the new testcase. Can you add some test cases? SHOW INDEX ON shidx_t1; right now lists all indexes on the given table? Can we also support show one index? i mean sth like : 'show index index_name on tbl' (you can do it in a follow up jira) Right now the show index output format is: +idx_name tab_name col_name idx_tab_name idx_type comment + + +idx_1 show_idx_full key default__show_idx_full_idx_1__ compact +idx_2 show_idx_full value1 default__show_idx_full_idx_2__ compact +idx_3 show_idx_full value2 default__show_idx_full_idx_3__ compact John, is this intentional? Otherwise, the patch looks good.
            jvs John Sichi added a comment -

            Right, we decided to do more details for a single index in a separate DESCRIBE command. Russell, please open a followup JIRA for that one now, link it to this one as related, and then edit the title for this one.

            jvs John Sichi added a comment - Right, we decided to do more details for a single index in a separate DESCRIBE command. Russell, please open a followup JIRA for that one now, link it to this one as related, and then edit the title for this one.
            jvs John Sichi added a comment -

            Regarding the output format, none of the other SHOW commands currently produce column headers, and we probably won't be able to modify them for backwards-compatibility reasons. What we ended up doing for DESCRIBE was to introduce DESCRIBE FORMATTED with the prettier format.

            Let's do the same for SHOW INDEXES. If we see SHOW FORMATTED INDEXES, produce the column headers; without FORMATTED, then omit them. (We can open a separate JIRA for doing that retroactively for the existing SHOW commands.)

            Also: add a test case for a compound index (multiple columns in the key), and change that header to col_names plural.

            jvs John Sichi added a comment - Regarding the output format, none of the other SHOW commands currently produce column headers, and we probably won't be able to modify them for backwards-compatibility reasons. What we ended up doing for DESCRIBE was to introduce DESCRIBE FORMATTED with the prettier format. Let's do the same for SHOW INDEXES. If we see SHOW FORMATTED INDEXES, produce the column headers; without FORMATTED, then omit them. (We can open a separate JIRA for doing that retroactively for the existing SHOW commands.) Also: add a test case for a compound index (multiple columns in the key), and change that header to col_names plural.
            jvs John Sichi added a comment -

            Oops, the followup already exists (HIVE-1764). I'll edit the title of this one.

            jvs John Sichi added a comment - Oops, the followup already exists ( HIVE-1764 ). I'll edit the title of this one.
            rmelick Russell Melick added a comment -
            • Removed commented out code
            • Added a test case with index comment
            • Added FORMATTED keyword to optionally show column headers
            • Added test case for compound index

            Yongqiang, I'm not sure why we want to take out the line

            {nopanel}
            setFetchTask(createFetchTask(showIndexesDesc.getSchema()));{nopanel}

            The other functions in the SemanticAnalyzer have similar function calls.

            John, we get the column headers directly from the schema we set inside of ShowIndexesDesc, so it would be difficult to pluralize col_name. Do you know what other show commands do?

            rmelick Russell Melick added a comment - Removed commented out code Added a test case with index comment Added FORMATTED keyword to optionally show column headers Added test case for compound index Yongqiang, I'm not sure why we want to take out the line {nopanel} setFetchTask(createFetchTask(showIndexesDesc.getSchema()));{nopanel} The other functions in the SemanticAnalyzer have similar function calls. John, we get the column headers directly from the schema we set inside of ShowIndexesDesc, so it would be difficult to pluralize col_name. Do you know what other show commands do?
            jvs John Sichi added a comment -

            @Russell: I meant pluralize it unconditionally (i.e. just edit the schema in your patch).

            jvs John Sichi added a comment - @Russell: I meant pluralize it unconditionally (i.e. just edit the schema in your patch).
            rmelick Russell Melick added a comment -

            Pluralized col_names in the FORMATTED column header

            rmelick Russell Melick added a comment - Pluralized col_names in the FORMATTED column header
            jvs John Sichi added a comment -

            For idx_compound, the col_names should be "key,value1" not just "key".

            jvs John Sichi added a comment - For idx_compound, the col_names should be "key,value1" not just "key".
            rmelick Russell Melick added a comment -

            fixed problem with only showing first column/key name

            rmelick Russell Melick added a comment - fixed problem with only showing first column/key name
            jvs John Sichi added a comment -

            +1. Will commit when tests pass.

            You could have used MetaStoreUtils.getColumnNamesFromFieldSchema, but I'll put a cleanup note for that on HIVE-1764 so we can get this one committed as is (assuming tests pass).

            jvs John Sichi added a comment - +1. Will commit when tests pass. You could have used MetaStoreUtils.getColumnNamesFromFieldSchema, but I'll put a cleanup note for that on HIVE-1764 so we can get this one committed as is (assuming tests pass).
            jvs John Sichi added a comment -

            Committed. Thanks Russell!

            jvs John Sichi added a comment - Committed. Thanks Russell!

            People

              rmelick Russell Melick
              jvs John Sichi
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: