Skip to content

Issue 2118 #2320

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

Merged
merged 5 commits into from
Apr 22, 2025
Merged

Issue 2118 #2320

merged 5 commits into from
Apr 22, 2025

Conversation

mzitnik
Copy link
Contributor

@mzitnik mzitnik commented Apr 21, 2025

This pull request introduces support for handling default values in ClickHouse columns, including MATERIALIZED, ALIAS, and EPHEMERAL types. It adds new functionality to skip these columns during data insertion and ensures proper parsing and testing of default expressions. The most important changes are grouped below:

Enhancements to ClickHouseColumn class:

  • Added a new DefaultValue enum to represent different default types (DEFAULT, MATERIALIZED, EPHEMERAL, ALIAS) and associated fields defaultValue and defaultExpression to the ClickHouseColumn class.
  • Introduced getter and setter methods for defaultValue and defaultExpression in the ClickHouseColumn class.

Changes to data insertion logic:

  • Updated the insert method in Client.java to skip columns with default values other than DEFAULT during data insertion.
  • Modified the commitRow method in RowBinaryFormatWriter.java to similarly skip columns with MATERIALIZED, ALIAS, or other non-default values.

Schema parsing improvements:

  • Enhanced TableSchemaParser to parse default_type and default_expression properties from table schema and set them in ClickHouseColumn.

New test cases:

  • Added a test case testWriterWithMaterialize to validate insertion behavior with columns having various default types (MATERIALIZED, ALIAS, EPHEMERAL).
  • Introduced insertSimplePOJOsWithMaterializeColumn test to verify POJO insertion with MATERIALIZED and ALIAS columns.

Additional utility class:

  • Added SimplePOJO class with a method to generate table creation SQL for testing columns with default values and default expressions.## Summary

Closes #2118 #2025

Checklist

Delete items not relevant to your PR:

@mzitnik mzitnik requested review from chernser and Paultagoras April 21, 2025 09:04
@@ -106,6 +106,21 @@ public final class ClickHouseColumn implements Serializable {
private Map<Class<?>, Integer> mapKeyToVariantOrdNumMap;
private Map<Class<?>, Integer> mapValueToVariantOrdNumMap;

public enum DefaultValue {
DEFAULT("Default"),
MATERIALIZED("MATERIALIZED"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be like Default - "Materialized" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

EPHEMERAL("EPHEMERAL"),
ALIAS("ALIAS");

public final String defaultValue;
Copy link
Contributor

Choose a reason for hiding this comment

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

May be lets rename it to kind or type just to avoid too many defaultValue in one place?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See my last comment, I prefer to stick with ClickHouse documentation language

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
54.3% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@mzitnik mzitnik merged commit a3f80b6 into main Apr 22, 2025
24 of 25 checks passed
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.

client.insert(...) fails if registered class doesn't match all existing columns
3 participants