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

Mechanism in OGR core to prevent creating duplicate field names? #11060

Open
rouault opened this issue Oct 20, 2024 · 6 comments
Open

Mechanism in OGR core to prevent creating duplicate field names? #11060

rouault opened this issue Oct 20, 2024 · 6 comments

Comments

@rouault
Copy link
Member

rouault commented Oct 20, 2024

Stepping back a bit from #11059, I'm wondering if OGRLayer::CreateField() shouldn't rather be renamed ICreateField() and have a non-virtual method OGRLayer::CreateField() that calls driver-overridden ICreateField(). Before doing that it could do some sanity checks like checking that there is no duplicate field names. Although there are drivers that probably somehow support it (CSV ? I also believe that Parquet itself supports duplicate field names, although the OGR driver probably does not). So perhaps there should be some OLCSupportsDuplicateFieldNames capability to condition that test?

Perhaps expose the maximum number of columns too? Limitations on field names ?

@theroggy
Copy link
Contributor

theroggy commented Oct 21, 2024

Are there file formats that use case sensitive column names? If that would be the case this is also something to think about.

Probably the same logic is applicable to renaming column names. In this regard, I noticed that if you rename a column while only changing the casing via an ALTER TABLE it isn't renamed. To do this you first need to rename to a really different name and then to the name with different casing.

@jratike80
Copy link
Collaborator

Are there file formats that use case sensitive column names?

PostGIS certainly, I think Oracle too and other "real" databases except SQLite.

@theroggy
Copy link
Contributor

theroggy commented Oct 21, 2024

Are there file formats that use case sensitive column names?

PostGIS certainly, I think Oracle too and other "real" databases except SQLite.

In Oracle this is not the case, unless you put double quotes around a column name... Based on a quick google it is similar in PostGIS: case insensitive unless double quoted, but still slightly different to PostGIS.
Now I remember that MS SQL Server does have case sensitive naming (or at least had 15 years ago).

But indeed, I didn't think about the "double quoting" case...

@jratike80
Copy link
Collaborator

jratike80 commented Oct 21, 2024

...unless you put double quotes around a column name

But of course you put double quotes around identifiers always when you write code. Otherwise you must answer all the time on the user forums that why my queries fail with colums like my test or my-test-column. Or why select column from table works with PostGIS but not with Oracle, that by default is using upper case.

Disclaimer: I do not write code but I do write those answers.

@theroggy
Copy link
Contributor

theroggy commented Oct 21, 2024

...unless you put double quotes around a column name

But of course you put double quotes around identifiers always when you write code. Otherwise you must answer all the time on the user forums that why my queries fail with colums like my test or my-test-column. Or why select column from table works with PostGIS but not with Oracle, that by default is using upper case.

For generic tools like gdal,... I agree: in geofileops I also double-quote in all queries to avoid the issues you raise.

For standard application development we (at least in our organisation) never do this as you are never supposed to use keywords or these kinds of characters in column names... and so all column names are treated case insensitive...

This is probably why I didn't think of oracle as an example of (possibly) being case sensitive... stupid of me.

@rouault
Copy link
Member Author

rouault commented Oct 21, 2024

OGRLayer::CreateField() shouldn't rather be renamed ICreateField() and have a non-virtual method OGRLayer::CreateField() that calls driver-overridden ICreateField(

on second thought, and looking at my actual #11059, things might be more complicated than that. For example duplicate name determination should happen after potential field name laundering. But field name laundering, if any, is per-driver specific

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

No branches or pull requests

3 participants