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: SYS TABLES adjustments for ODBC: TYPE argument #30398

Closed
bpintea opened this issue May 4, 2018 · 6 comments
Closed

SQL: SYS TABLES adjustments for ODBC: TYPE argument #30398

bpintea opened this issue May 4, 2018 · 6 comments
Assignees

Comments

@bpintea
Copy link
Contributor

bpintea commented May 4, 2018

Elasticsearch version: master @ 65dbc17

With TYPE argument now working (as a fix for https://github.com/elastic/x-pack-elasticsearch/issues/4335), there are these two follow-up issues that have emerged:

  • some applications use TABLE as filter (like Microsoft Query:'TABLE','VIEW','SYNONYM'). This will not match BASE TABLE and the result is empty. My proposal would be to make the two - BASE TABLE and TABLE - interchangeable for this query (even though they aren't, technically), unless we do actually differentiate between these two concepts.
  • The ODBC spec mandates that the TYPE parameter, if not an empty string, [it] must contain a list of comma-separated values for the types of interest; each value can be enclosed in single quotation marks (') or unquoted, for example, 'TABLE', 'VIEW' or TABLE, VIEW.. Current ES/SQL implementation will accept the quoted list, but won't accept the unquoted format. I haven't yet encountered apps that make use of the unquoted format. The fix for this could obviously be implemented in the driver too -- up for discussion.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@costin
Copy link
Member

costin commented May 5, 2018

  1. Can you please check why TABLE is being used and whether that's not a bug on our side? Normally the client should query for the types supported by the driver and then ask for additional information.
    Microsoft Query seems to support both base table and derived table hence why it's a bit weird to see it ask for TABLE without any differentiation.

  2. We cannot support unquoted values due to the SQL grammar. That is the table types are interpreted as literals and per SQL need to be quoted ('). Not using quotes means we need to change the grammar to add a dedicated keyword for each value, which is excessive and redundant.
    So I think this needs to handled inside the driver - which might have more context on the actual value and how to convert it to a literal.

@bpintea
Copy link
Contributor Author

bpintea commented May 7, 2018

  1. Can you please check why TABLE is being used and whether that's not a bug on our side?

After looking into msqry and delving through postgresql's and mysql's drivers, I've found out that actually BASE TABLE would not work at all as argument for this catalog function: they expect TABLE only here. Confused (and too late in the process), I checked the spec again, which states indeed that this argument can only be one of the following: "TABLE", "VIEW", "SYSTEM TABLE", "GLOBAL TEMPORARY", "LOCAL TEMPORARY", "ALIAS", "SYNONYM", or a data source–specific type name..

So, here again, just as for point 2., we have the option of either changing the plugin implementation or handle this in the driver. For the record, both drivers "intercept" this argument and adapt it to the server.

Also for the record, on their respective CLIs, both psql and mysql report the table types as BASE TABLE. So there's a clear differentiation between the concepts and the drivers take care of adapting the results.

The follow-up question would then be: besides the TABLE to BASE TABLE one, should the driver do any other translation?

  1. So I think this needs to handled inside the driver

OK, will take care of it there.

@costin
Copy link
Member

costin commented May 8, 2018

For the record, both drivers "intercept" this argument and adapt it to the server.

Interesting. Do you know what the complete mapping of psql is? How do they handle BASE TABLE vs DERIVED TABLE? As TABLE vs VIEW?

@bpintea
Copy link
Contributor Author

bpintea commented May 8, 2018

Postresql's ODBC driver will only allow return results for SYSTEM TABLE, TABLE, VIEW, as standard table types, and MATVIEW and FOREIGN TABLE, as their specific type.
Anything else will be ignored; this is explicitly commented in the code: NOTE: Unsupported table types (i.e., LOCAL TEMPORARY, ALIAS, etc) will return nothing.

In a setup where MS Query connects to MS SQL Server through FreeTDS (over the TDS protocol), the driver will not "intercept" the type parameter - as postgresql's one does -, but simply quote whatever tokens it receives, if they aren't quoted (so addressing the issue described in point 2 above).
For an ODBC SQLTables() call, besides a plethora of system tables, SQL Server returns a user-defined table as of type TABLE (so matching what's been queried for!), even if a direct SELECT (using Management Studio) from the database's information_schema will return the BASE TABLE type.

This is in line with postgresql behaviour too:

$ psql testdb -E
psql (9.1.24lts2)
Type "help" for help.

testdb=# \d
********* QUERY **********
SELECT n.nspname as "Schema",
  c.relname as "Name",
  CASE c.relkind WHEN 'r' THEN 'table' WHEN 'v' THEN 'view' WHEN 'i' THEN 'index' WHEN 'S' THEN 'sequence' WHEN 's' THEN 'special' WHEN 'f' THEN 'foreign table' END as "Type",
  pg_catalog.pg_get_userbyid(c.relowner) as "Owner"
FROM pg_catalog.pg_class c
     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace
WHERE c.relkind IN ('r','v','S','f','')
      AND n.nspname <> 'pg_catalog'
      AND n.nspname <> 'information_schema'
      AND n.nspname !~ '^pg_toast'
  AND pg_catalog.pg_table_is_visible(c.oid)
ORDER BY 1,2;
**************************

        List of relations
 Schema |  Name  | Type  | Owner
--------+--------+-------+-------
 public | t_test | table | root
(1 row)

testdb=# SELECT table_type FROM information_schema.TABLES WHERE TABLE_NAME like '%test%';
 table_type
------------
 BASE TABLE
(1 row)

Note: r in relkind / pg_class stands for an "ordinary table". Their ODBC driver translates to this value r as well when servicing SQLTables() -- the query the driver generates is: select relname, nspname, relkind from pg_catalog.pg_class c, pg_catalog.pg_namespace n where relkind in ('r', 'v') and nspname not in ('pg_catalog', 'information_schema', 'pg_toast', 'pg_temp_1') and n.oid = relnamespace order by nspname, relname.

My conclusion of this: in general, if an application needs to differentiate between the types of tables available into a data source, it would have to query the information schema "directly", via an explicit SELECT statement. As far as ODBC's SQLTables() is concerned, the specific type of table - BASE, DERIVED etc. - is irrelevant, unless it's a SYSTEM table or data source specific.
I believe that the issue we have with this is that we're missing now the possibility to query the information schema via a SELECT and SYS TABLES is the only query available to get this data.

The options I would see:

  • have the driver handle the types of tables;
  • extend SYS TABLES with a new parameter that would decide if a BASE TABLE (or other table types we might add) is returned as such or just TABLE.
  • have SYS TABLES always report the different types of tables as just TABLE.

@bpintea
Copy link
Contributor Author

bpintea commented May 9, 2018

Issue described in .2 above addressed in bpintea/elasticsearch-sql-odbc@13f4883

costin added a commit that referenced this issue May 10, 2018
Support TABLE as a legacy argument for SYS TABLE commands

Fix #30398
costin added a commit to costin/elasticsearch that referenced this issue May 10, 2018
Support TABLE as a legacy argument for SYS TABLE commands

Fix elastic#30398
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants