Skip to content

[CALCITE-6584] Validate prefixed column identifiers in SET clause of UPDATE statement#3972

Closed
nielspardon wants to merge 1 commit intoapache:mainfrom
nielspardon:par-6584
Closed

[CALCITE-6584] Validate prefixed column identifiers in SET clause of UPDATE statement#3972
nielspardon wants to merge 1 commit intoapache:mainfrom
nielspardon:par-6584

Conversation

@nielspardon
Copy link
Copy Markdown
Contributor

@nielspardon nielspardon commented Sep 19, 2024

This PR adds validation of prefixed column identifiers in SET clause of UPDATE statement (and in the UPDATE statement within a MERGE statement)

  • This is a follow-up PR to [CALCITE-6576] SqlParser: allow using table alias with column identifiers in SET section of UPDATE statement #3959
  • I first had to change SqlValidatorUtil.addFields() to use the Util.last() method for getting the last element of a compound identifier instead of relying on simple identifiers
  • I then implemented a validation logic in SqlValidatorImpl.createTargetRowType() which is being used for validating INSERT, UPDATE and MERGE
  • in presence of the new targetTableAlias parameter the validation logic checks the prefix of any non-simple identifiers or in other words it checks that any prefixed column identifiers are prefixed with the target table alias
  • I tried to check for any existing validation logic that I could reuse and it's possible I might have missed a utility method that I could have used.
  • The validation logic supports a mix of prefixed and unprefixed columns in the SET clause. Not sure if I should only allow prefixed columns here.

Let me know your thoughts.

…UPDATE statement

Signed-off-by: Niels Pardon <par@zurich.ibm.com>
@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Member

@caicancai caicancai left a comment

Choose a reason for hiding this comment

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

LGTM

@Test void testAliasInSetClauseOfUpdate() {
// good examples
sql("UPDATE sales.emp AS e SET e.deptno = 10").ok();
sql("UPDATE emp AS e SET e.deptno = 10").ok();
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.

postgres actually rejects this: Query Error: error: column "e" of relation "emp" does not exist

julianhyde added a commit to julianhyde/calcite that referenced this pull request Sep 24, 2024
github-actions Bot pushed a commit that referenced this pull request Sep 25, 2024
…UPDATE statement

Signed-off-by: Niels Pardon <par@zurich.ibm.com>

Close #3972
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants