Skip to content

HIVE-28446: Convert some reserved words to non-reserved words - #5394

Merged
deniskuzZ merged 7 commits into
apache:masterfrom
okumin:HIVE-28446-non-reserved
Oct 28, 2024
Merged

HIVE-28446: Convert some reserved words to non-reserved words#5394
deniskuzZ merged 7 commits into
apache:masterfrom
okumin:HIVE-28446-non-reserved

Conversation

@okumin

@okumin okumin commented Aug 14, 2024

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Mark some reserved words as non-reserved based on the ANSI standard. I did the following.

  1. Write unit tests using the existing parser
  2. Pick up keywords that are not reserved in the ANSI standard
  3. Add reserved words which are not declared in the WIKI to the non-reserved list

Why are the changes needed?

We found some extra reserved keywords have been added while we were testing Hive 2 -> Hive 4. I guess some of them are intentionally reserved, but some of them are unintentionally reserved.

One point. We're not biased toward making everything non-reserved. It is OK to discard this PR and add all keywords to the WIKI.

Does this PR introduce any user-facing change?

Users can use the changed words without adding quotes.

This PR would not tighten any restrictions, so I assume this is backward compatible.

Is the change a dependency upgrade?

No.

How was this patch tested?

Added unit tests, and checked the behavior of Trino and PostgreSQL.

trino> create table hive.default.test_keywords (APPLICATION int, COMPACTIONID int, CONF int, CONNECTOR int, CONNECTORS int, DATABASE int, DDL int, EXCHANGE int, EXTENDED int, FOLLOWING int, FORCE int, IF int, LESS int, MACRO int, MORE int, OLDER int, PKFK_JOIN int, PRECEDING int, SYNC int, THAN int, TIMESTAMPLOCALTZ int, UNBOUNDED int);
CREATE TABLE
postgres=# create table test_keywords (APPLICATION int, COMPACTIONID int, CONF int, CONNECTOR int, CONNECTORS int, DATABASE int, DDL int, EXCHANGE int, EXTENDED int, FOLLOWING int, FORCE int, IF int, LESS int, MACRO int, MORE int, OLDER int, PKFK_JOIN int, PRECEDING int, SYNC int, THAN int, TIMESTAMPLOCALTZ int, UNBOUNDED int);
CREATE TABLE

| KW_PKFK_JOIN
| KW_THAN
| KW_TIMESTAMPLOCALTZ
;

@okumin okumin Aug 15, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tickets and fix versions

@okumin
okumin marked this pull request as ready for review August 15, 2024 03:25
| KW_TIMESTAMPLOCALTZ -> TOK_TIMESTAMPLOCALTZ
//| KW_TIMESTAMPTZ -> TOK_TIMESTAMPTZ
| KW_TIMESTAMP KW_WITH KW_LOCAL KW_TIME KW_ZONE -> TOK_TIMESTAMPLOCALTZ
//| KW_TIMESTAMP KW_WITH KW_TIME KW_ZONE -> TOK_TIMESTAMPTZ

@deniskuzZ deniskuzZ Aug 28, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we drop this as well since TOK_TIMESTAMPTZ doesn't exist?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll be right. I removed it.
03207b0

Also, I found some more extra words while addressing HIVE-19741. I cleaned them up in the same way.
a808727

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.

@github-actions github-actions Bot added the stale label Oct 28, 2024
@deniskuzZ deniskuzZ removed the stale label Oct 28, 2024

@deniskuzZ deniskuzZ left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM +1

@deniskuzZ
deniskuzZ merged commit d6cb698 into apache:master Oct 28, 2024
@okumin
okumin deleted the HIVE-28446-non-reserved branch October 28, 2024 11:22
@okumin

okumin commented Oct 28, 2024

Copy link
Copy Markdown
Contributor Author

Thanks

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