add a way to exclude a column from schema validation#816
add a way to exclude a column from schema validation#816gavinking wants to merge 1 commit intojakartaee:mainfrom
Conversation
gavinking
force-pushed
the
column-validated
branch
4 times, most recently
from
2800fbc to
7770576
Compare
| * Determines if this column is validated as part of the | ||
| * schema {@linkplain SchemaManager#validate validation} | ||
| * process. By default, the column is validated. If the | ||
| * column should be excluded from validation, specify | ||
| * {@code validated=false}. |
There was a problem hiding this comment.
There is an important ambiguity here. What does "excluded from validation" mean precisely? Does it mean we check that the column exists but perform no further validation beyond that? Or does it mean we don't even validate that it exists in the table? 🤔
There was a problem hiding this comment.
I can't imagine a situation where one would want their program to run against a database where one of the column doesn't exists... ?
The original use case was to workaround limitations in validation type-checking. So I think we want that:
we check that the column exists but perform no further validation beyond that?
There was a problem hiding this comment.
Yeah, that's probably right.
sometimes users run into a case where the schema validation is too strict, and their program actually works as is; it's nice to have a way to exclude a column from validation see jakartaee#815
gavinking
force-pushed
the
column-validated
branch
from
7770576 to
a38dd43
Compare
| * Determines if this column is validated as part of the | ||
| * schema {@linkplain SchemaManager#validate validation} | ||
| * process. By default, the column is validated. If the | ||
| * column should be excluded from validation, specify | ||
| * {@code validated=false}. |
There was a problem hiding this comment.
I can't imagine a situation where one would want their program to run against a database where one of the column doesn't exists... ?
The original use case was to workaround limitations in validation type-checking. So I think we want that:
we check that the column exists but perform no further validation beyond that?
There was a problem hiding this comment.
Doesn't validation also make sense for things that are not related to columns?
I couldn't find a precise definition of schema validation in the JPA spec, so I suppose it's up to the implementation to pick what gets validated.
I can see that there are a few more things than columns which could impact the schema:
@Table.uniqueConstraints@Table.indexes@Table.check
And maybe more but the rest seems... unlikely to be validated.
Maybe @Table.validated would make some sort of sense?
There was a problem hiding this comment.
I mean, yeah, in principle, but I'm cautious about adding features to the spec for some purely-speculative feature that some JPA implementation might have at some unspecified point in the future.
So unless we're making a decision to start validating the existence of constraints, I wouldn't add this.
That's not to say I don't think constraint validation would be useful. Just we don't have any implementation of it yet.
There was a problem hiding this comment.
The other side of the coin is that if we don't add it now, then when an implementation decides to implement such validation (since that's up to the implementation), there will likely be a delay of months, likely years, before the corresponding "validation" exclusion is made available for tables in the same way it is available for columns. Leading to either a gap in functionality, or the temporary introduction of native annotations -- which, for @Table at least, we've been trying to get rid of.
One way to circumvent your position would be to decree that validated = false on a table means both the table's constraints and its columns don't get validated (regardless of what's configured at the column level). Then the attribute would have a use right now.
Otherwise, okay, if it's a conscious decision, fair enough.
There was a problem hiding this comment.
I guess I can sorta see that validated=false is a reasonably natural way to say "don't validate the existence of columns". (Though we agree that's not especially useful.)
I'm not sure it's an especially good way to indicate that, say, a foreign key or unique key constraint should not be validated. I feel like that's something you would want to be able to set per-constraint, not once for the whole table.
Also note that @ForeignKey actually already has something pretty close to that with @ForeignKey(NO_CONSTRAINT). Ignore the particular syntax, which I've never liked.
There was a problem hiding this comment.
* @since 2.1
*/
public enum ConstraintMode {@since 2.1 is this massive red flag in the persistence APIs. 😭
There was a problem hiding this comment.
I feel like that's something you would want to be able to set per-constraint, not once for the whole table.
True, but then the whole feature is a workaround to deal with false-positives of validation. Disabling validation of a specific constraint would be best, but failing that disabling validation of the whole table still has benefits (you can validate other tables).
2 participants
Sometimes users run into a case where the schema validation is too strict, and their program actually works as is; it's nice to have a way to exclude a column from validation.
Fixes #815.
WDYT?