Skip to content

HIVE-26355: Column compare should be case insensitive for name#3406

Merged
pvary merged 2 commits intoapache:masterfrom
wecharyu:HIVE-26355
Jul 1, 2022
Merged

HIVE-26355: Column compare should be case insensitive for name#3406
pvary merged 2 commits intoapache:masterfrom
wecharyu:HIVE-26355

Conversation

@wecharyu
Copy link
Contributor

What changes were proposed in this pull request?

Fix a bug in comparing column fields between old table and new table.

Why are the changes needed?

Hive is case-insensitive in field name, but some engines are case-sensitive, so when calling alter_table the existing column name may not be equal because of the case.

Does this PR introduce any user-facing change?

No

How was this patch tested?

We add two unit tests, can be run with command:

$ mvn test -Dtest=org.apache.hadoop.hive.metastore.utils.TestMetaStoreServerUtils -pl :hive-standalone-metastore-server

@wecharyu
Copy link
Contributor Author

@pvary @nrg4878 : Could you review this PR?

@pvary
Copy link
Contributor

pvary commented Jun 30, 2022

Maybe we should convert the incoming column names "immediately" to lower case, otherwise other methods like (arePrefixColumns) would suffer from the same comparison issue. What do you think?

@wecharyu
Copy link
Contributor Author

I have also considered to convert the column names at the top method like alter_table, but since there are only two methods (areSameColumns and arePrefixColumns) that will take use of the compare of names, so I made the conversion in these two methods.

@pvary
Copy link
Contributor

pvary commented Jun 30, 2022

I have also considered to convert the column names at the top method like alter_table, but since there are only two methods (areSameColumns and arePrefixColumns) that will take use of the compare of names, so I made the conversion in these two methods.

Could you please add a javadoc which describes that the methods are case independent?

@wecharyu
Copy link
Contributor Author

Could you please add a javadoc which describes that the methods are case independent?

Done.

@pvary pvary merged commit c4dc616 into apache:master Jul 1, 2022
DongWei-4 pushed a commit to DongWei-4/hive that referenced this pull request Oct 28, 2022
dengzhhu653 pushed a commit to dengzhhu653/hive that referenced this pull request Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants