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

Fix load relative file for LoadDataChange, CreateProcedureChange or CreateViewChange #2073

Merged
merged 9 commits into from Dec 16, 2021

Conversation

smainz
Copy link

@smainz smainz commented Sep 3, 2021

Environment

Liquibase Version: 4.4.3 (and all 4.x before)

Liquibase Integration & Version: any

Liquibase Extension(s) & Version: liquibase-core

Database Vendor & Version: H2 (and all others)

Operating System Type & Version: Windows 10 (and all others)

Pull Request Type

Description

See #1277, but for LoadDataChange, CreateProcedureChange or CreateViewChange.
Testcases showing the problem are included in this PR.

Same as #2060 but with different branch name. My branch name had chars which broke the build.

Steps To Reproduce

List the steps to reproduce the behavior.

  • create a changelog file with these change sets
<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
				     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
				     xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
                      http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.10.xsd"
				     logicalFilePath="a-logical-file-path"
>
	  <changeSet id="1" author="author">
		  <loadData file="data/some_data.csv" tableName="a_tabe"
				   relativeToChangelogFile="true"></loadData>
	  </changeSet>

	  <changeSet id="2" author="author">
		  <createProcedure path="sql/proc.sql"
						   relativeToChangelogFile="true"></createProcedure>
	  </changeSet>

	  <changeSet id="3" author="author">
		  <createView path="sql/view.sql" viewName="a_view"
					  relativeToChangelogFile="true"></createView>
	  </changeSet>

</databaseChangeLog>
  • Create these files (may be empty)
    • data/some_data.csv
    • sql/proc.sql
    • sql/view.sql
  • Run liquibase update against the db
liquibase --driver=com.microsoft.sqlserver.jdbc.SQLServerDriver --url="jdbc:sqlserver://localhost:1433;database=the_database" --changeLogFile=changelog.xml  --username=the_username --password=thepassword --logLevel=debug update
  • See it fail
  • Comment the changeset and repeat (fails for all three)

Actual Behavior

Liquibase throws an exception complaining about missing file (a different one for each changeset):

liquibase.exception.LiquibaseException: liquibase.exception.UnexpectedLiquibaseException: File 'data/some_data.csv' not found

Expected/Desired Behavior

Liquibase does not complain about missing files and works flawlessly.

Additional Context

Same bug has already been fixed in #1798 for a diffenent change. The changes above have been overlooked.

I had to fix an existing testcase which used a changeset not included in an changelog.

Fast Track PR Acceptance Checklist:

  • Build is successful and all new and existing tests pass (all in liquibase.core)
  • Added [Unit Test(s)] Enhanced existing unit tests
  • Added [Integration Test(s)]
  • Added Test Harness Test(s)
  • Documentation Updated

Dev Handoff Notes (Internal Use)

Links

Testing

Dev Verification

Created a changeset with the above example changes and saw they failed in 4.6.2 with "cannot find file" errors. With the PR, they all correctly find the files.

Test Requirements (Internal Liquibase QA)

  • Create these files (examples are attached)
    • data/names.csv
    • sql/proc.sql
    • sql/view.sql

Manual Test Requirements
Verify update is successful with load relative files.

  • liquibase update --changelog-file lb-2179-changelog.xml
  • Update is executed successfully
  • No exception is thrown
  • Table populated exists
  • table populated contains data from names.csv
  • View someview exists
  • Procedure SomeProcedure exists

┆Issue is synchronized with this Jira Bug by Unito

@nvoxland
Copy link
Contributor

I pushed the current version of master to your fork to fix the build issue.

@molivasdat
Copy link
Contributor

Hi @smainz Thanks for creating this PR.

A member of the Liquibase team will take a look at your contribution and may suggest:

The PR will be prioritized according to our internal development and testing capacity.

We’ll let you know when it’s ready to move to the next step or if any changes are needed.

@molivasdat molivasdat added BBroad Broad Issue DBAll DBH2 ImpactLow IntegrationAny RiskMedium Changes that require more testing or that affect many different code paths. Severity3 TypeBug labels Sep 16, 2021
@molivasdat molivasdat added this to To Do in Conditioning++ via automation Sep 16, 2021
@smainz
Copy link
Author

smainz commented Nov 5, 2021

Is there anything I can help to get this PR reviewed/approved?

@nvoxland nvoxland moved this from To Do to Code Review in Conditioning++ Dec 8, 2021
@nvoxland
Copy link
Contributor

nvoxland commented Dec 8, 2021

Thanks you for the very good PR!

@suryaaki2 suryaaki2 moved this from Code Review to Ready for Handoff (In JIRA) in Conditioning++ Dec 8, 2021
@sync-by-unito sync-by-unito bot changed the title Fix load ralative file Fix load relative file Dec 9, 2021
@sync-by-unito
Copy link

sync-by-unito bot commented Dec 13, 2021

➤ Erzsebet Carmean commented:

To reproduce this bug, it is necessary to have a . in the directory path where the changelog is located. This is similar to https://datical.atlassian.net/browse/LB-2178 ( https://datical.atlassian.net/browse/LB-2178|smart-link ) (Liquibase Internal ticket), which maps to the GH PR #2066 ( https://github.com/liquibase/liquibase/pull/2066|smart-link ) .

@suryaaki2 suryaaki2 merged commit 34287cb into liquibase:master Dec 16, 2021
Conditioning++ automation moved this from Ready for Handoff (In JIRA) to Done Dec 16, 2021
@nvoxland nvoxland changed the title Fix load relative file Fix load relative file for LoadDataChange, CreateProcedureChange or CreateViewChange Jan 6, 2022
@nvoxland nvoxland removed this from Done in Conditioning++ Jan 6, 2022
@nvoxland nvoxland added this to the v4.7.0 milestone Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BBroad Broad Issue DBAll DBH2 ImpactLow IntegrationAny RiskMedium Changes that require more testing or that affect many different code paths. Severity3 SyncTicket TypeBug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Child of #1277: Error loading relative file when logicalFilePath is set (Breaks migration from 3.x to 4.x)
5 participants