HHH-18784 TableMigrator shouldn't alter column length if length is not explicitly-set

[quaff]

[Please describe here what your change is about]

---

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-18784

[quaff]

@gavinking WDYT?

[gavinking]

@gavinking WDYT?

I'm not sure I agree with the concept.

We don't recommend running TableMigrator on your production database, so it's a tool mainly for use in more development-oriented contexts.

In that context I would say it would be annoying if adding @Column(length=20) to my entity would not result in any change.

But, well, this merits further discussion.

[quaff]

@gavinking WDYT?

I'm not sure I agree with the concept.

We don't recommend running TableMigrator on your production database, so it's a tool mainly for use in more development-oriented contexts.

In that context I would say it would be annoying if adding @Column(length=20) to my entity would not result in any change.

But, well, this merits further discussion.

It's annoying development too, It's kind of regression because I can do that in early version.

[gavinking]

I don't see how it can be annoying in development. Why would you want to develop against a schema that disagrees with what you have explicitly specified in your mapping annotations?

If there is disagreement there, I would want to know about it!

[quaff]

I don't see how it can be annoying in development. Why would you want to develop against a schema that disagrees with what you have explicitly specified in your mapping annotations?

If there is disagreement there, I would want to know about it!

I didn’t specify @column explicitly, the length defaults to 255

[quaff]

Perhaps we should check the required length is default length

[gavinking]

Perhaps we should check the required length is default length

Yeah, I was wondering that too.

Actually, @sebersole, I believe we now have ways to distinguish an unset/defaulted length from an explicitly-set length=255, is that right?

[quaff]

Perhaps we should check the required length is default length

Yeah, I was wondering that too.

Actually, @sebersole, I believe we now have ways to distinguish an unset/defaulted length from an explicitly-set length=255, is that right?

@gavinking Please review the updated PR, column will not be modified if @Column is not explicitly-set, but there is no way to distinguish between @Column and @Column(length=255).

[gavinking]

there is no way to distinguish between @column and @column(length=255).

I believe that Steve now has a way to do that in hibernate-models.

[quaff]

there is no way to distinguish between @column and @column(length=255).

I believe that Steve now has a way to do that in hibernate-models.

I inspected org.hibernate.boot.models.annotations.internal.ColumnJpaAnnotation, I don't think hibernate-models can handle this currently, It should retain a map of explicit attributes.

[sebersole]

I believe that Steve now has a way to do that in hibernate-models.

No, I initially did, when using Jandex (which does understand this distinction). But everyone else disagreed with that so I rempoved it.

[gavinking]

Well, OK, then, given that we can't distinguish an explicit length=255, I guess I'm just -1 on making this change.

[sebersole]

It should retain a map of explicit attributes.

The problem is the same. We don't know which values are explicit (again, unless using Jandex). Java makes default and eplicit values indistinguishable.

[sebersole]

Rejecting this one

[quaff]

Rejecting this one

I think this PR is worthy to merge, It distinguish @Column present or not at least.

[gavinking]

@quaff We never treat a missing annotation as different to a missing annotation member for the JPA mapping annotations.

[quaff]

@quaff We never treat a missing annotation as different to a missing annotation member for the JPA mapping annotations.

I agree that, but I'm requesting special treat for schema migration here, the unit tests in this PR should pass.

[gavinking]

And that's precisely the sort of ad hoc hacked-in special case which I'm thoroughly against. How am I supposed to explain to a user that "oh, it behaves one way with the @Column annotation but a different way without it", even though the actual inferred schema is identical. That's terrible. There's no way the user would expect that behavior.

Again, schema migration is a limited tool with very limited capabilities, which is supposed to assist at development time. It's not a replacement for something like Flyway or whatever, and we've never promoted it as such.