Skip to content

Conversation

@namuuCY
Copy link

@namuuCY namuuCY commented Dec 17, 2025

Refactor InformixDialect to override the new getSelectClauseNullString(SqlTypedMapping, TypeConfiguration) signature.

Instead of relying on the legacy integer-based sqlType logic, this change utilizes the DdlTypeRegistry to dynamically determine the correct cast type name. This ensures better compatibility and aligns the dialect with the standard Hibernate 6 API for null handling in select clauses.

Changes

  • Override getSelectClauseNullString(SqlTypedMapping, TypeConfiguration) in InformixDialect.
  • Use DdlTypeRegistry to resolve the cast type string.

Note on scope

Initially, this PR attempted to address columnDefinition usage for PostgreSQL. However, based on the discussion with @gavinking, we identified that the PostgreSQL issues could be resolved via proper mapping (@JdbcTypeCode(SqlTypes.INET) or InetAddress).
Therefore, per the agreement, I have dropped the PostgreSQL changes and columnDefinition logic from this PR and focused solely on refactoring InformixDialect to use the modern API.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.


https://hibernate.atlassian.net/browse/HHH-19974

Copy link
Member

@gavinking gavinking left a comment

Choose a reason for hiding this comment

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

please roll back all the unrelated whitespace changes so that we can see the substance of the proposed change

.getCastTypeName( sqlType.toSize(), (SqlExpressible) sqlType.getJdbcMapping(), ddlTypeRegistry );
// PostgreSQL assumes a plain null literal in the select statement to be of type text,
// which can lead to issues in e.g. the union subclass strategy, so do a cast
String castTypeName = sqlType.getColumnDefinition();
Copy link
Member

Choose a reason for hiding this comment

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

I guess that this is the substance of the proposed change?

But this doesn't look right to me. If this getColumnDefinition() method returns the content of @Column(columnDefinition), then that is not a column type and should never be treated as such.

Copy link
Author

Choose a reason for hiding this comment

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

You are absolutely right.

If columnDefinition contains constraints like default ... or not null, passing it directly to cast() will indeed cause a syntax error. I missed that edge case.

The underlying issue is: The field is mapped as String in Java, so Hibernate currently defaults to casting the null literal as varchar. However, the actual DB column is inet, and PostgreSQL throws a type mismatch error (UNION types inet and character varying cannot be matched).

Since we can't safely use columnDefinition, is there a recommended way in Hibernate to extract only the SQL type name (e.g. inet) for these native types, or should we register these types explicitly in the Dialect to avoid the varchar fallback?

Copy link
Member

Choose a reason for hiding this comment

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

You can probably do it by registering a DdlType and a JdbcType. Something like that.

Copy link
Member

Choose a reason for hiding this comment

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

I mean it looks like we already have this stuff built in:

You just write:

public InetAddress addr;

Or if you want to use a string:

@JdbcTypeCode(SqlTypes.INET) public String addrStr;
Copy link
Author

Choose a reason for hiding this comment

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

@gavinking

I have verified your suggestions.

You are absolutely right regarding the inet type.
I confirmed that both approaches work perfectly without any changes to the dialect:

  1. Using @JdbcTypeCode(SqlTypes.INET) on a String field.
  2. Using the InetAddress Java type directly (e.g., private InetAddress addr;).

In both cases, the test passes and Hibernate correctly generates cast(null as inet) in the SQL.
So, at least for types that can be mapped to a standard SqlTypes (or have a built-in mapping like InetAddress), the current mechanism works fine if the mapping is configured correctly.

Copy link
Author

Choose a reason for hiding this comment

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

Also, regarding InformixDialect:

I noticed that InformixDialect currently only overrides the deprecated getSelectClauseNullString(int sqlType, TypeConfiguration typeConfiguration) method.

Even if we drop the columnDefinition logic (as agreed), would you be open to a PR that refactors InformixDialect to override the new getSelectClauseNullString(SqlTypedMapping sqlType, TypeConfiguration typeConfiguration) signature instead?

@Override public String getSelectClauseNullString(SqlTypedMapping sqlType, TypeConfiguration typeConfiguration) { final DdlTypeRegistry ddlTypeRegistry = typeConfiguration.getDdlTypeRegistry(); final String castTypeName = ddlTypeRegistry	.getDescriptor( sqlType.getJdbcMapping().getJdbcType().getDdlTypeCode() )	.getCastTypeName( sqlType.toSize(), (SqlExpressible) sqlType.getJdbcMapping(), ddlTypeRegistry ); return "cast(null as " + castTypeName + ")"; }

This would align the dialect with the modern API and make future type handling improvements easier.

Copy link
Member

Choose a reason for hiding this comment

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

Even if we drop the columnDefinition logic (as agreed), would you be open to a PR that refactors InformixDialect to override the new getSelectClauseNullString(SqlTypedMapping sqlType, TypeConfiguration typeConfiguration) signature instead?

Yes, sure, thanks.

@namuuCY
Copy link
Author

namuuCY commented Dec 18, 2025

@gavinking
Cleaned up the whitespace!

PostgreSQLLegacyDialectTest.InetEntity.class,
PostgreSQLLegacyDialectTest.EmptyEntity.class
})
@SessionFactory
Copy link
Member

Choose a reason for hiding this comment

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

Please use the standard statement collector instead. Same for the other PostgreSQL test

Suggested change
@SessionFactory
@SessionFactory(useCollectingStatementInspector = true)
Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the review.
Based on the discussion with Gavin above, we decided to drop the PostgreSQL changes from this PR and focus solely on the Informix refactoring.

Therefore, I will be removing the PostgreSQL-related test files in the next commit, so this change won't be applied. I'll update the PR shortly.

…eNullString signature. Instead of relying on columnDefinition logic, this change utilizes the DdlTypeRegistry to dynamically determine the correct cast type name, ensuring better compatibility and correctness for null handling in select clauses on Informix. Signed-off-by: namucy <wkdcjfdud13@gmail.com>
@namuuCY namuuCY changed the title HHH-19974 Use columnDefinition for null casting HHH-19974 Refactor InformixDialect to override getSelectClauseNullString Dec 18, 2025
@namuuCY namuuCY requested review from beikov and gavinking December 18, 2025 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants