What's a valid identifier?

[MaxKsyunz]

SQL plugin uses identifiers in several different places. In each case we try to be as liberal as possible but sometimes that results in unexpected parser behaviour.

With support for different catalogs on the way, each with unique identifier rules, now is a good time to write down what constitutes a valid identifier in SQL plugin.

These are the different namespaces involved:

1. ANSI SQL [haven't found a good reference for this.]
2. MySQL
3. OpenSearch
4. S3
5. Prometheus

Here're a few cases to consider. For each case,

1. When can identifiers be left unquoted?
2. When must they be quoted?

<placeholder> denotes an identifier.

Catalog name

SELECT * FROM <catalog>.tableA

Table name

SELECT * FROM <table>

Column name

SELECT <columnName> FROM tableA

Column alias

SELECT columnA AS <alias> FROM tableA

Table alias

SELECT columnA FROM tableA AS <tableAlias>

SQL standard function call

SELECT <function>(....) FROM tableA

Catalog-specific function call

`SELECT * FROM tableA WHERE (....)

User-defined function call

SELECT <function>(...) FROM tableA

[MaxKsyunz]

My two cents.

If an identifier is from a catalog namespace and is a valid ANSI SQL/MySQL identifier, then it can be left unquoted, otherwise it must be surrounded by back ticks. This applies to table name, column name, and catalog-specific functions.

If an identifier is for a SQL construct, then it follows ANSI SQL/MySQL rules. This applies to table aliases, column aliases, and user-defined functions.

If an identifier is a plugin-specific construct, then it follows ANSI SQL/MySQL rules.

This would be a breaking change, but it's easy to understand.

[dai-chen]

@MaxKsyunz Thanks for bringing up this topic!

In each case we try to be as liberal as possible but sometimes that results in unexpected parser behaviour.

Could you give more details or link some issues here? I recall there are some issue due to same identifier rule for table and column. But I'm not sure if you're referring to the same problem.

With support for different catalogs on the way, each with unique identifier rules, now is a good time to write down what constitutes a valid identifier in SQL plugin.

Currently SQL identifier is defined in ANTLR grammar. No matter which catalog, any name mismatched has to be quoted. Are you proposing to extend what can be matched by current identifier rule to accommodate S3 and Prometheus?

If an identifier is from a catalog namespace ...

What do you mean by coming from a catalog? As I understand in ANSI SQL, every database object is under a schema of a catalog/database.

[MaxKsyunz]

Could you give more details or link some issues here? I recall there are some issue due to same identifier rule for table and column.
But I'm not sure if you're referring to the same problem.

Here're two use cases I'm thinking of.

First, in MySQL (and ANSI SQL) in statements like SELECT fieldA*fieldB from tableA and SELECT fieldA-fieldB from tableA, -, and * are always treated as math operators.

That's the expected behaviour for users of the JDBC driver. However OpenSearch SQL plugin will treat fieldA*fieldB and fieldA-fieldB as column names.

Another scenario -- with support for catalogs, the statement SELECT * FROM prometheus.indexA can refer to either table prometheus.indexA or table indexA in prometheus catalog. As far as I can see, for any connector id, one can construct such ambiguous scenario. If I'm not mistaken, even quoting the table name SELECT * FROM `prometheus.indexA` will check if prometheus is a catalog name.

With support for different catalogs on the way, each with unique identifier rules, now is a good time to write down what constitutes a valid identifier in SQL plugin.

Currently SQL identifier is defined in ANTLR grammar. No matter which catalog, any name mismatched has to be quoted. Are you proposing to extend what can be matched by current identifier rule to accommodate S3 and Prometheus?

I propose we restrict unquoted identifiers to the safest-possible regex (I think ANSI SQL's [A-Za-z][A-Z_a-z0-9]*will work) and require the rest to be quoted. This will solve the math operator issue and is-it-a-catalog issue and make the behaviour of SQL statements more predictable.

If an identifier is from a catalog namespace ...

What do you mean by coming from a catalog? As I understand in ANSI SQL, every database object is under a schema of a catalog/database.
Yes. I wanted to make a distinction between database object names and identifiers created by the user of the system -- aliases and SQL parameter names currently.

[dai-chen]

Got it. For this issue alone, I think this is a bug. The intention was to allow * present only in index name (to align with index pattern expression supported by OpenSearch). But unfortunately we’ve used same ANTLR rule for both index name and field name.

We can double check if any real use case or IT for this. If not, this can be a quick fix. For others, we need to discuss further and make breaking change in next major version if needed.

[MaxKsyunz]

Here's another question. In OpenSearch, SELECT ... FROM index* would select from all indices that start with index. We get this from OpenSearch, if I'm not mistaken.

What happens when we have, say a Prometheus connector and execute SELECT ... FROM prometheus.index*?

[Yury-Fridlyand]

There are limitations for index name: link. So, test_index!@^'[]{}() is a valid index name. We can't allow this as an identifier, such names should be quoted with backticks ( ` ).
I think we should follow ANSI SQL:

An ordinary identifier must begin with a letter and contain only letters, underscore characters (_), and digits

[dblock]

Interesting topic! I think the plugin should expose an implementation of some type of SQL that's unambiguously defined, i.e. ANSI SQL (or another standard), and follow the identifier conventions of that standard, and not OpenSearch. That of course assumes that the standard can be a superset of OpenSearch identifiers, such as through quoting.

The other way around (identifiers are defined by OpenSearch) would mean that some parts of the language are borrowed from one system (e.g. index* as identifier), while others are borrowed from the standard (e.g. SELECT *) which can be ambiguous as @Yury-Fridlyand rightfully points out above.

[MaxKsyunz]

Quoting seems the safest especially given incoming support for different catalogs.

How much of the current syntax and behaviour is specific to OpenSearch and will break for other catalogs like S3 and Prometheus.

Prime example -- SELECT * FROM open*. It's a valid query that returns documents from all indices that start with open. We get this behaviour from OpenSearch. What happens with non-OpenSearch catalogs? Will SELECT * FROM opensearch.open* and SELECT * FROM Prometheus.open* have the same behaviour?