-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
geo: allow case insensitive SRID= at the start of EWKT #48551
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @otan)
pkg/geo/parse.go, line 194 at r1 (raw file):
} for i := 0; i < len(prefix); i++ { if unicode.ToLower(rune(str[i])) != unicode.ToLower(rune(prefix[i])) {
It is not clear to me why this is correct in the general utf-8 case (though fine for SRID): my understanding is that the string index returns a byte, not a rune. Can you fix that or add a comment that this works only for a prefix that is ascii.
pkg/geo/parse.go, line 194 at r1 (raw file): Previously, sumeerbhola wrote…
I'll just cop it and write the builtin myself. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 3 of 3 files at r2.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @otan)
pkg/geo/parse.go, line 194 at r1 (raw file):
Previously, otan (Oliver Tan) wrote…
I'll just cop it and write the builtin myself.
Can you add the following to the function level comment.
// It assumes that the prefix contains only ASCII bytes.
pkg/util/strings.go, line 34 at r2 (raw file):
} // ToLowerSingleByte returns the the lowercase of a given single ASCII byte.
can you add
// A non ASCII byte is returned unchanged.
and add a couple of non-ascii bytes to the test.
PostGIS allows any combination of `srid` at the beginning of the string. Follow the same allowance by having a case insensitive HasPrefix. Release note: None
tftr! bors r=sumeerbhola |
Build succeeded |
PostGIS allows any combination of
srid
at the beginning of the string.Follow the same allowance by having a case insensitive HasPrefix.
Release note: None