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

OGR SQL: add SELECT expression [AS] OGR_STYLE HIDDEN to be able to specify feature style string without generating a visible column #10260

Merged
merged 3 commits into from
Jun 22, 2024

Conversation

rouault
Copy link
Member

@rouault rouault commented Jun 20, 2024

This is a bit inspired by the HIDDEN keyword used by some other SQL implementations, but generally in other contexts, such as hidden columns in SQLite3 virtual tables (https://www.sqlite.org/vtab.html) or MySQL invisible columns (https://dev.mysql.com/doc/refman/8.4/en/invisible-columns.html), although those are in a CREATE TABLE context.

Fixes #10259

@elpaso / @dbaston opinions?

Other alternatives I considered by decreasing order of appeal:

  • having OGRGenSQLResultsLayer consider a field name "OGR_STYLE_HIDDEN" to mean "take that field value to set SetStyleString() but do not expose it"
  • having some config option (OGR_STYLE_HIDDEN=YES) read by OGRGenSQLResultsLayer to do the same
  • always hide OGR_STYLE (but only when it is used in the form "expression AS OGR_STYLE", not just "OGR_STYLE" from source layer), although this has the potential for breaking some workflows
  • per-driver options (no, we definitely don't want that)

One downside of the implementation is that it makes 'HIDDEN' a reserved keyword, so any field/table with that name must now be quoted, although I could probably tune the BNF to accept 'HIDDEN' as an identifier as I don't think there would be grammar ambiguity in doing so (to be tested...)

I finally selected the HIDDEN SQL keyword because we might potentially use it for other situations (potentially to do things like "SELECT my_id_field AS FID HIDDEN, ..." to set the FID (we can currently select the source fid with SELECT FID ... but not set it)

@rouault rouault force-pushed the fix_10259 branch 5 times, most recently from 59a0b07 to 7619587 Compare June 20, 2024 22:38
@coveralls
Copy link
Collaborator

Coverage Status

coverage: 69.251% (+0.005%) from 69.246%
when pulling 7619587 on rouault:fix_10259
into 082d0e7 on OSGeo:master.

@coveralls
Copy link
Collaborator

Coverage Status

coverage: 69.248% (+0.002%) from 69.246%
when pulling 9b9f8d7 on rouault:fix_10259
into 082d0e7 on OSGeo:master.

@coveralls
Copy link
Collaborator

Coverage Status

coverage: 69.25% (+0.004%) from 69.246%
when pulling 1ab895d on rouault:fix_10259
into 082d0e7 on OSGeo:master.

Copy link
Member

@dbaston dbaston left a comment

Choose a reason for hiding this comment

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

having some config option (OGR_STYLE_HIDDEN=YES) read by OGRGenSQLResultsLayer to do the same

This seems the simplest to me, but the argument about adding the same behavior for FID is a good one.

doc/source/user/ogr_sql_dialect.rst Outdated Show resolved Hide resolved
ogr/ogrsf_frmts/generic/ogr_gensql.cpp Outdated Show resolved Hide resolved
ogr/ogrsf_frmts/generic/ogr_gensql.cpp Outdated Show resolved Hide resolved
ogr/ogrsf_frmts/generic/ogr_gensql.cpp Outdated Show resolved Hide resolved
ogr/ogrsf_frmts/generic/ogr_gensql.cpp Show resolved Hide resolved
ogr/ogrsf_frmts/generic/ogr_gensql.cpp Outdated Show resolved Hide resolved
@rouault rouault force-pushed the fix_10259 branch 3 times, most recently from c949a85 to 1636ccb Compare June 21, 2024 17:48
…ecify feature style string without generating a visible column

This is a bit inspired by the HIDDEN keyword used by some other SQL
implementations, but generally in other contexts, such as hidden columns
in SQLite3 virtual tables (https://www.sqlite.org/vtab.html) or
MySQL invisible columns
(https://dev.mysql.com/doc/refman/8.4/en/invisible-columns.html),
although those are in a CREATE TABLE context.

Fixes OSGeo#10259
@coveralls
Copy link
Collaborator

Coverage Status

coverage: 69.25% (+0.004%) from 69.246%
when pulling 0d006d1 on rouault:fix_10259
into 082d0e7 on OSGeo:master.

@coveralls
Copy link
Collaborator

Coverage Status

coverage: 69.246%. remained the same
when pulling a577383 on rouault:fix_10259
into 082d0e7 on OSGeo:master.

@rouault rouault merged commit 2a3af36 into OSGeo:master Jun 22, 2024
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LIBKML OGR_STYLE
3 participants