Skip to content

HHH-20092 SqlTypes.JSON on SQL Server expects varchar(max) even with use_nationalized_character_data=true#11680

Closed
qlfyd123 wants to merge 14 commits intohibernate:mainfrom
qlfyd123:HHH-20092
Closed

HHH-20092 SqlTypes.JSON on SQL Server expects varchar(max) even with use_nationalized_character_data=true#11680
qlfyd123 wants to merge 14 commits intohibernate:mainfrom
qlfyd123:HHH-20092

Conversation

@qlfyd123
Copy link
Copy Markdown
Contributor

@qlfyd123 qlfyd123 commented Jan 26, 2026

https://hibernate.atlassian.net/browse/HHH-20092
[Please describe here what your change is about]

Added a descriptor to DdlTypeRegistry to map json to nvarchar.


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.



Please make sure that the following tasks are completed:
Tasks specific to HHH-20092 (Bug):

  • Add test reproducing the bug
  • Add entries as relevant to migration-guide.adoc OR check there are no breaking changes

Copy link
Copy Markdown
Member

@mbellade mbellade left a comment

Choose a reason for hiding this comment

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

Thanks @qlfyd123, left a couple comments on how to approach this.

@qlfyd123
Copy link
Copy Markdown
Contributor Author

@mbellade
I’ve re-examined the issue and identified the root cause: the Dialect was missing a descriptor registration, causing Hibernate to always interpret SQL Server’s JSON as a VARCHAR type. Consequently, I have removed the previous logic and added the appropriate descriptor to the SQL Server Dialect.

@qlfyd123 qlfyd123 requested a review from mbellade January 29, 2026 02:06
@qlfyd123
Copy link
Copy Markdown
Contributor Author

qlfyd123 commented Feb 3, 2026

Hi @mbellade.
I introduced logic to apply AdjustableJdbcType adaptation even when an explicit JdbcType is provided in InferredBasicValueResolver.
The primary goal was to ensure that JDBC types correctly adapt to metadata like @Nationalized or global property
However, this change causes a regression in LengthTest#testSqlType

  • Cause: LongVarcharJdbcType (used for LONG32VARCHAR) implements AdjustableJdbcType. When resolveIndicatedType is called without an explicit length (using default Length.LONG), it downgrades the type code from LONG32VARCHAR
    to the standard LONGVARCHAR.
  • Result: H2 Dialect maps LONGVARCHAR to VARCHAR(32600) instead of CLOB, causing a "Value too long" exception when inserting large data. Before this change, the explicit LONG32VARCHAR was preserved, leading to a correct CLOB
    mapping.

I would appreciate guidance on the correct design direction:
Is it the intended behavior to apply AdjustableJdbcType logic to explicitly defined JDBC types?
If we should NOT apply adaptation logic to explicit types, how should we handle JSON on SQL Server? Should we treat it like other DBs and use a single fixed type (e.g., always NVARCHAR(MAX)) regardless of the nationalized attribute, effectively ignoring the attribute for explicit JSON types?

I'm happy to adjust the implementation based on your feedback. For now, I've pushed the changes to facilitate discussion despite the test failure.

@beikov
Copy link
Copy Markdown
Member

beikov commented Feb 5, 2026

Given that the JSON data type or alternative we're using on all other databases is able to store all characters (not just ASCII), I think it might be best to default to add the following to the SQLServerDialect:

		jdbcTypeRegistry.addDescriptor( JsonAsStringJdbcType.NVARCHAR_INSTANCE );
		jdbcTypeRegistry.addDescriptor( XmlAsStringJdbcType.NVARCHAR_INSTANCE );

Or maybe even do that in the AbstractTransactSQLDialect since Sybase ASE probably has a similar problem.

Instead of validating the column type, can you please write a test that stores a unicode character into a JSON string and verify that it comes out the same way again? See StringNationalizedMappingTests as an example.
That way, we will also understand immediately when running the testsuite which database mappings are incorrect.

@qlfyd123
Copy link
Copy Markdown
Contributor Author

Hi @beikov
I registered JsonAsStringJdbcType.NVARCHAR_INSTANCE and XmlAsStringJdbcType.NVARCHAR_INSTANCE in AbstractTransactSQLDialect
and added a test that stores and retrieves a Unicode character in a JSON string.
Could you please take another look when you have a chance? Let me know if you need any further adjustments.

Copy link
Copy Markdown
Member

@beikov beikov left a comment

Choose a reason for hiding this comment

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

Please make sure this single test JsonUnicodeMappingTests works on all databases.

@qlfyd123
Copy link
Copy Markdown
Contributor Author

@beikov
I update the test code for all database tests.

@qlfyd123
Copy link
Copy Markdown
Contributor Author

@beikov
I remove old test and apply override method

@beikov
Copy link
Copy Markdown
Member

beikov commented Mar 17, 2026

It seems we will need to merge #11989 first to get rid of the failure for Sybase.

@qlfyd123
Copy link
Copy Markdown
Contributor Author

@beikov
I applied the jTDS driver patch for Sybase to the supportsNationalizedMethods() method usage section.

Copy link
Copy Markdown
Member

@beikov beikov left a comment

Choose a reason for hiding this comment

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

Thanks. Please also apply consistent formatting.

@qlfyd123
Copy link
Copy Markdown
Contributor Author

@beikov I revert STREAM_BINDING binder change and fix import formatting.

Copy link
Copy Markdown
Member

@beikov beikov left a comment

Choose a reason for hiding this comment

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

The formatting is still a bit off. Note that we usually have a space character before and after parenthesis, as well as the ? and : ternary operator parts.

@qlfyd123
Copy link
Copy Markdown
Contributor Author

@beikov Sorry about that — I re-applied the formatting.
I have a quick question.I ran the spotless command before committing, but some formatting changes weren't applied. Am I misunderstanding how spotless works? I ran command by gradlew :hibernate-core:spotlessApply

@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
4.5% Coverage on New Code (required ≥ 80%)
34.7% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@beikov beikov self-requested a review March 26, 2026 16:30
@beikov
Copy link
Copy Markdown
Member

beikov commented Mar 27, 2026

I'm sorry this takes so long, but the jTDS driver was posing a problem which wasn't easy to solve. I cleaned up the code now and created a new PR: #12105

@beikov beikov closed this Mar 27, 2026
@qlfyd123 qlfyd123 deleted the HHH-20092 branch March 30, 2026 13:09
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.

3 participants