Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[plsql] updates should allow for multiple statements #1554

Merged
merged 1 commit into from
Jan 20, 2019

Conversation

tashiscool
Copy link

@tashiscool tashiscool commented Jan 3, 2019

Before submitting a PR, please check that:

  • The PR is submitted against master. The PMD team will merge back to support branches as needed.
  • ./mvnw clean verify passes. This will build and test PMD, execute PMD and checkstyle rules. Check this for more info

PR Description:

@jsotuyod jsotuyod changed the title *[plsql]* updates should allow for multiple statements [plsql] updates should allow for multiple statements Jan 4, 2019
@oowekyala
Copy link
Member

Hi and thanks for the contribution! Could you remove the whitespace changes though? Your actual changes are lost in their noise, I have no idea what you modified.

@jsotuyod
Copy link
Member

jsotuyod commented Jan 5, 2019

@oowekyala you can add ?w=1 to the url to ignore whitespace in the diff, ie: https://github.com/pmd/pmd/pull/1554/files?w=1

@oowekyala
Copy link
Member

oowekyala commented Jan 5, 2019

@jsotuyod so cool, thanks! Then do ignore my comment tashiscool

@pmd-test
Copy link

pmd-test commented Jan 9, 2019

1 Warning
⚠️ This PR cannot be merged yet.

Generated by 🚫 Danger

Copy link
Member

@adangel adangel left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!
I've created a corresponding bug report: #1583

Please see my question - I'm unsure whether the combination with ROW and columns is actually allowed in plsql.

@@ -1904,8 +1904,10 @@ ASTUpdateSetClause UpdateSetClause() :
<SET>
(
<ROW> "=" ID()
( "," Column() "=" ( Expression() | <_DEFAULT> )*)*
Copy link
Member

Choose a reason for hiding this comment

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

Is this correct? I didn't find a clear description, whether a ROW can be combined with single columns at the same time. The ROW uses a record, which contains all columns anyway...

So: Is this actually valid syntax?

UPDATE schedule
  SET ROW = default_week,
    Tue = 'Day Off'
  WHERE week = 1;

Copy link
Member

Choose a reason for hiding this comment

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

I just tested it, and I get this one:

ORA-06550: line 16, column 27:
PL/SQL: ORA-00933: SQL command not properly ended
ORA-06550: line 15, column 5:
PL/SQL: SQL Statement ignored
06550. 00000 -  "line %s, column %s:\n%s"
*Cause:    Usually a PL/SQL compilation error.

So, this combination is not possible....

@adangel adangel self-assigned this Jan 19, 2019
@adangel adangel merged commit f6c62ef into pmd:master Jan 20, 2019
@adangel adangel added this to the 6.11.0 milestone Jan 20, 2019
adangel added a commit that referenced this pull request Jan 20, 2019
@adangel
Copy link
Member

adangel commented Jan 20, 2019

Thanks for your contribution! This is merged and will be part of PMD 6.11.0.

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.

6 participants