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

MissingDataChangeGenerator: small optimisation, skip null-value columns (issues #6420) #6422

Open
wants to merge 64 commits into
base: master
Choose a base branch
from

Conversation

catull
Copy link
Contributor

@catull catull commented Oct 14, 2024

Export table data preserving NULL-values, or not.

Impact

  • Enhancement/New feature (adds functionality without impacting existing logic)

Description

Have you ever wondered if changelog exports could skip all NULL-values ?
This MR does exactly this.

It implements issue #6420.

Export table data with or without NULL-values.
- GenerateChangeLogPostgresIntegrationTest updated.
…te tests against Postgres DB.

- Include missing changelog tests are using.
@catull
Copy link
Contributor Author

catull commented Dec 13, 2024

Hey @catull,

Could you please review the changes made on the serializer, I think they are not yet fixing these cases. For example what has been added in XMLChangeLogSerializer:

if (value instanceof ColumnConfig) {
      ColumnConfig columnConfig = (ColumnConfig) value;
      if (!preserveNullValues && columnConfig.isNull()) {
          return;
      }
      node.appendChild(createNode((LiquibaseSerializable) value));
}

is broken the below assertion from serialize_pretty_nestedNodeWithAttributesAndNotExistsIgnored() (XMLChangeLogSerializerTest)

assertEquals("<createTable catalogName=\"a\"\n" +
                "        schemaName=\"b\"\n" +
                "        tableName=\"c\">\n" +
                "    <column defaultValue=\"x1\" name=\"x\"/>\n" +
                "    <column defaultValue=\"y1\" name=\"y\"/>\n" +
                "</createTable>", out);

Because we are not including defaultValue in the node as it was behaving previously. Additionally, it would be worth checking other serializers to see if the same issue is happening. At the moment, we don't have any reported issues for those serializers related to this change, but that's because we don't have specific tests for them.

Thanks, Daniel.

I adapted the code to only emit value="null" where necessary.
Please review it, thank you.

@catull
Copy link
Contributor Author

catull commented Dec 24, 2024

@MalloD12 Can you please review the changes ? Thanks.

@catull catull requested a deployment to external December 26, 2024 19:35 — with GitHub Actions Abandoned
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Discussion
Development

Successfully merging this pull request may close these issues.

MissingDataChangeGenerator: small optimisation, skip null-value columns in export
2 participants