Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL: Suppress geo tests failing on tr-TR locale #42200

Merged

Conversation

imotov
Copy link
Contributor

@imotov imotov commented May 17, 2019

Due to a bug in JTS WKT parser, JTS cannot parse most of WKT shapes if
the shape type is written in the lower case. For examples point (1 2)
is causing JTS inside H2GIS to fail on tr-TR locale as a result of
case-insensitive comparison. I will try to fix this issue in JTS as a follow
up.

Due to a bug in JTS WKT parser, JTS cannot parse most of WKT shapes if
the shape type is written in the lower case. For examples `point (1 2)`
is causing JTS inside H2GIS to fail as a result of case-insensitive
comparision. I will try to fix this issue in JTS as a follow up.
@imotov imotov added >test Issues or PRs that are addressing/adding tests :Analytics/Geo Indexing, search aggregations of geo points and shapes :Analytics/SQL SQL querying v8.0.0 v7.2.0 labels May 17, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@astefan
Copy link
Contributor

astefan commented May 17, 2019

Just as a side comment: tr-TR is not the only locale with a slightly different behavior. In total I think there are 8.

Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

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

In a similar fashion in other place in code we do this differently - https://github.com/elastic/elasticsearch/blob/master/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/SqlSpecTestCase.java#L69 - but I guess is ok in this case to have a comparison with the uppercase version of point.

@imotov
Copy link
Contributor Author

imotov commented May 17, 2019

@astefan converting point to upper case is pretty much the only operation that messes things up in this case. However, I ran the test on all 8 locales listed in the linked suppression and the check suppressed tests on all 8 of them here as well. So, I think until new locales that uppercase latin letters in a different from English way show up, both checks are functionally equivalent.

@imotov imotov merged commit e63b1fb into elastic:master May 17, 2019
imotov added a commit that referenced this pull request May 17, 2019
Due to a bug in JTS WKT parser, JTS cannot parse most of WKT shapes if
the shape type is written in the lower case. For examples `point (1 2)`
is causing JTS inside H2GIS to fail on tr-TR locale as a result  of 
case-insensitive comparison.
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
Due to a bug in JTS WKT parser, JTS cannot parse most of WKT shapes if
the shape type is written in the lower case. For examples `point (1 2)`
is causing JTS inside H2GIS to fail on tr-TR locale as a result  of 
case-insensitive comparison.
@imotov imotov deleted the suppress-geosql-tests-on-turkish-locale branch May 1, 2020 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Geo Indexing, search aggregations of geo points and shapes :Analytics/SQL SQL querying >test Issues or PRs that are addressing/adding tests v7.2.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants