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

Fixed Formatted SQL "property" parsing #3037

Merged
merged 2 commits into from Jul 12, 2022

Conversation

nvoxland
Copy link
Contributor

@nvoxland nvoxland commented Jul 1, 2022

Impact

  • Bug fix (non-breaking change which fixes expected existing functionality)
  • Enhancement/New feature (adds functionality without impacting existing logic)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

A comment that has the word "property" in it anywhere like -- I don't know what this property does breaks formatted sql parsing with an Formatted SQL changelogs require known formats, such as '--property name=<property name> value=<property value>' and others to be recognized and run. error.

Fixes DAT-10720

Things to be aware of

  • Only impacts how we parse for --property lines in formatted sql
  • Existing tests look for correct forms, and they continue to pass

Things to worry about

  • Nothing

@github-actions
Copy link

github-actions bot commented Jul 1, 2022

Unit Test Results

  4 560 files  ±  0    4 560 suites  ±0   32m 28s ⏱️ + 1m 20s
  4 540 tests +  1    4 322 ✔️ +  1     218 💤 ±0  0 ±0 
53 772 runs  +12  48 760 ✔️ +12  5 012 💤 ±0  0 ±0 

Results for commit eea140d. ± Comparison against base commit bd71f9f.

♻️ This comment has been updated with latest results.

@StevenMassaro
Copy link
Contributor

Is it possible to add a test around this?

Copy link
Contributor

@XDelphiGrl XDelphiGrl left a comment

Choose a reason for hiding this comment

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

  • Fix addresses a bug where formatted SQL keywords that were not at the beginning of a line broke changelog parsing.
    • Keywords include property, changeset, rollback, precondition, comment, and ignoreLines, validCheckSum.
    • New unit tests added to FormattedSqlChanegLogParser for all of the keywords, in a variety of locations in a line.
  • No additional testing required.

APPROVED

Passing Functional Tests
Passing Test Harness Execution

@kataggart
Copy link
Contributor

Is it possible to add a test around this?

@nvoxland did you see this question on this one?

@kataggart kataggart added this to the NEXT milestone Jul 12, 2022
@nvoxland
Copy link
Contributor Author

Yes, I added a tests for it @kataggart

@nvoxland nvoxland merged commit 6f1717f into master Jul 12, 2022
@nvoxland nvoxland deleted the fix-formattedsql-property-parsing branch July 12, 2022 15:41
@nvoxland nvoxland linked an issue Jul 27, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Liquibase sql comment validation bug
4 participants