Skip to content

HIVE-29570: Fix MERGE rewrite parse failure by correctly quoting qual…#6439

Open
kokila-19 wants to merge 1 commit intoapache:masterfrom
kokila-19:fix_parse_error
Open

HIVE-29570: Fix MERGE rewrite parse failure by correctly quoting qual…#6439
kokila-19 wants to merge 1 commit intoapache:masterfrom
kokila-19:fix_parse_error

Conversation

@kokila-19
Copy link
Copy Markdown
Contributor

…ified column names when they are function names(like date)

What changes were proposed in this pull request?

During merge rewrite, quote the qualified identifiers when those identifiers are same as function name.

Why are the changes needed?

Hive rewrites a MERGE into a different SQL statement (typically INSERT, SELECT) and then parses the rewritten SQL again.
That rewritten SQL is produced from the AST.
After HIVE-29187, the unparse logic does not quote any identifier that matches a built‑in function name.

So when the rewritten query contains a qualified reference like: s.date the unparse step must regenerate it as: 's' .'date'
Instead, it was 's'.date
Because it was not quoted, it fails to parse when the rewritten query is compiled.

Does this PR introduce any user-facing change?

No

How was this patch tested?

mvn test -pl itests/qtest -Dtest=TestMiniLlapLocalCliDriver -Dqfile=sqlmerge.q -Dtest.output.overwrite=true -Pitests

@kokila-19 kokila-19 marked this pull request as ready for review April 17, 2026 11:33
@kokila-19 kokila-19 changed the title HIVE-29570: Fix MERGE rewrite parse failure by correctly quoting qual… WIP: HIVE-29570: Fix MERGE rewrite parse failure by correctly quoting qual… Apr 17, 2026
Comment on lines +364 to +370
ASTNode parent = (ASTNode) identifier.getParent();
if (parent != null
&& (parent.getType() == HiveParser.TOK_FUNCTION
|| parent.getType() == HiveParser.TOK_FUNCTIONDI
|| parent.getType() == HiveParser.TOK_FUNCTIONSTAR)
&& parent.getChildCount() > 0
&& parent.getChild(0) == identifier) {
Copy link
Copy Markdown
Contributor

@kasakrisz kasakrisz Apr 20, 2026

Choose a reason for hiding this comment

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

I think this is a good solution.

The alternative would be to

  • remove the IdentifierProcessor
  • add separate processors for TOK_TABNAME, TOK_TABLE_OR_COL, TOK_FUNCTION etc.

The risk of this alternative is that we might miss an AST node.
So let's keep yours.

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.

That’s great. I’ve refactored the code into a helper method to improve readability.

Comment on lines +29 to +32
create table src_table(a int, `date` int) clustered by (a) into 2 buckets stored as orc
TBLPROPERTIES ('transactional'='true');
create table tgt_table(a int, `date` int) clustered by (a) into 2 buckets stored as orc
TBLPROPERTIES ('transactional'='true');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you please change one of the table names to a quoted function name.

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.

Updated the table name

@kokila-19 kokila-19 changed the title WIP: HIVE-29570: Fix MERGE rewrite parse failure by correctly quoting qual… HIVE-29570: Fix MERGE rewrite parse failure by correctly quoting qual… Apr 20, 2026
@kokila-19 kokila-19 requested a review from kasakrisz April 20, 2026 09:58
Copy link
Copy Markdown
Contributor

@kasakrisz kasakrisz left a comment

Choose a reason for hiding this comment

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

+1
please remove the unused imports from HiveUtils

…ified column names when they are function names(like date)
@sonarqubecloud
Copy link
Copy Markdown

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