HHH-19956: Automated contribution from Perforce //main/clients/Hibernate/7.1/...

[isc-service]

---

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-19956 (New Feature):

• Add tests for feature/improvement
• Update documentation as relevant: javadoc for changed API, documentation/src/main/asciidoc/userguide for all features, documentation/src/main/asciidoc/introduction for main features, links from existing documentation
• Add entries as relevant to migration-guide.adoc (breaking changes) and whats-new.adoc (new features/improvements)

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

[beikov]

You did cover a couple of changes that I requested, but left out quite a few. I did the work now to copy these comments over from #11595 to this PR, but this madness of creating new PRs must stop. Please answer questions that I raise on this PR and push changes to this PR instead of creating new ones. Otherwise, I can't review the changes properly.

[beikov]

Please remove that, we never use this type code as DDL type code anywhere.

Suggested change

case LONGVARBINARY:

[beikov]

Please remove that, we never use this type code as DDL type code anywhere.

Suggested change

case LONGVARCHAR:

[beikov]

I understand that you don't support a parameter for the binary(N) DDL type, but the super.columnType() call will handle VARBINARY already.

Suggested change

case VARBINARY:

[beikov]

According to your documentation, you do support the default double precision DDL type, so this should not be necessary.

Suggested change

	case DOUBLE:
	return "double";

[beikov]

According to your documentation, you do support the parameterized float(N) DDL type, so this should not be necessary.

Suggested change

	case FLOAT:
	return "float";

[beikov]

Why would users want to use this when there is the upper function in HQL?

[beikov]

Suggested change

	functionRegistry.namedDescriptorBuilder( "nullif" )
	.setExactArgumentCount( 2 )
	.setParameterTypes(ANY, ANY)
	.setReturnTypeResolver( useArgType( 1 ) )
	.register();
	functionFactory.nullif();

[beikov]

Suggested change

	functionRegistry.namedDescriptorBuilder( "lower" )
	.setExactArgumentCount( 1 )
	.setParameterTypes(STRING)
	.setInvariantType( stringType )
	.register();
	functionRegistry.namedDescriptorBuilder( "upper" )
	.setExactArgumentCount( 1 )
	.setParameterTypes(STRING)
	.setInvariantType( stringType )
	.register();
	functionFactory.lowerUpper();

[beikov]

Have you actually tested if this function can be used? I'm kind of doubting that a % sign is a valid java identifier start character.
I'm also curious about all the other functions that you list here is you tested them or if you have an actual need for them. Many of the built-in functions you list here probably have HQL "standard" alternatives which are much more likely to be used by people.

For example, HQL has the extract function which allows to extract parts like the day of month, day of week, etc. from a temporal expression. I'm not saying that you have to remove those function registrations, just that it seems unnecessary to bother with them, given that there are alternatives that are supposed to work across dialects.

[beikov]

Suggested change

	.setParameterTypes(ANY, ANY)
	.setReturnTypeResolver( useArgType( 1 ) )
	.setArgumentTypeResolver( StandardFunctionArgumentTypeResolvers.ARGUMENT_OR_IMPLIED_RESULT_TYPE )
	.setReturnTypeResolver( StandardFunctionReturnTypeResolvers.useFirstNonNull() )