search

opengeospatial / ets-gpkg12

GeoPackage 1.2 Executable Test Suite
Other
7 stars 10 forks source link

Column Data Types - Invalid data type for TEXT column #128

Closed phidrho closed 1 week ago

phidrho commented 6 months ago

Hi,

CITE reports FAIL on "column Data Types".

I have used TEXT columns in my GPKG with declaration of maxchar_count. According to 1.2.1 version of standard and current version 1.4 this should be allowed as described in Table 1.

The same table is referenced in CITE documentation for this test.

table1_gpkg

This should be eventually a warning for this case in my opinion if there is such status.

I am not aware if this topic was discussed before, if there is an agreement on this, please close bug report.

dstenger commented 6 months ago

Thank you for reporting. Do you have a test file which we can use to reproduce the error? If not, can you please document the exact error message?

phidrho commented 6 months ago

@dstenger I prepared an test file which causes Fail on Core > column Data Types with Reason:

Invalid data type: TEXT (500) for column text_data_limited in table my_attribute_table

Maxchar Count Bug example

bpross-52n commented 2 weeks ago

@phidrho The test suite takes the maxchar_count into account, but it is checked against a regular expression. See here However, this check does not expect a blank between TEXT and the opening bracket. Would it be possible for you to remove the blank in the datatype description, i.e. TEXT(500)?

jyutzler commented 2 weeks ago

@phidrho The test suite takes the maxchar_count into account, but it is checked against a regular expression. See here However, this check does not expect a blank between TEXT and the opening bracket. Would it be possible for you to remove the blank in the datatype description, i.e. TEXT(500)?

Wouldn't it make sense to adjust the regular expression instead?

dstenger commented 2 weeks ago

Sure, this might also be an option. We could truncate all white spaces. However, it has to be checked if this is compliant to the specification.

jyutzler commented 2 weeks ago

Sure, this might also be an option. We could truncate all white spaces. However, it has to be checked if this is compliant to the specification.

Yes, it is compliant to the specification. Thanks to datatype affinity (according to them it is a feature not a bug), the underlying engine doesn't even care about those particulars. Therefore, we should lean towards permissiveness.

I would entertain a corrigendum to modify the relevant part of the abstract test suite, if you think that is necessary.

phidrho commented 2 weeks ago

@phidrho The test suite takes the maxchar_count into account, but it is checked against a regular expression. See here However, this check does not expect a blank between TEXT and the opening bracket. Would it be possible for you to remove the blank in the datatype description, i.e. TEXT(500)?

It's not problem for me, now that I'm aware of it, but there are lots of automatic SQL formatters that might cause this. And I believe that it's allowed by SQL standard, because I never had trouble writing sintax this way, and I've used many different softwares for SQLite.