You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2020/07/07 11:55:21 UTC
[GitHub] [phoenix-queryserver] stoty opened a new pull request #42: PHOENIX-5994 SqlAlchemy schema filtering incorrect semantics
stoty opened a new pull request #42:
URL: https://github.com/apache/phoenix-queryserver/pull/42
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix-queryserver] stoty commented on a change in pull request #42: PHOENIX-5994 SqlAlchemy schema filtering incorrect semantics
Posted by GitBox <gi...@apache.org>.
stoty commented on a change in pull request #42:
URL: https://github.com/apache/phoenix-queryserver/pull/42#discussion_r451084403
##########
File path: python-phoenixdb/phoenixdb/tests/test_sqlalchemy.py
##########
@@ -52,6 +52,39 @@ def test_textual(self):
finally:
connection.execute('drop table if exists ALCHEMY_TEST')
+ def test_schema_filtering(self):
+ engine = self._create_engine()
+ with engine.connect() as connection:
+ try:
+ connection.execute('drop table if exists ALCHEMY_TEST')
+ connection.execute('drop table if exists A.ALCHEMY_TEST_A')
+ connection.execute('drop table if exists B.ALCHEMY_TEST_B')
+
+ connection.execute(text('create table ALCHEMY_TEST (ID integer primary key)'))
+ connection.execute(text('create table A.ALCHEMY_TEST_A (ID_A integer primary key)'))
+ connection.execute(text('create table B.ALCHEMY_TEST_B (ID_B integer primary key)'))
+
+ inspector = db.inspect(engine)
+
+ self.assertEqual(inspector.get_schema_names(), [None, 'A', 'B', 'SYSTEM'])
Review comment:
In most DBs the default scheme seems to have textual representation, so I couldn't find anything definitive by a cursory search.
At least we also return None from **sqlalchemy.engine.reflection.Inspector.default_schema_name** , so we are internally consistent.
I'll try to check how Hue handles this semantics tomorrow. (Should have done that before opening the PR)
I've run into the null = '' weirdness before, but it seems to work in direction only. (i.e. `column = ''` won't match on empty strings / null , only `is null` will)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix-queryserver] joshelser commented on a change in pull request #42: PHOENIX-5994 SqlAlchemy schema filtering incorrect semantics
Posted by GitBox <gi...@apache.org>.
joshelser commented on a change in pull request #42:
URL: https://github.com/apache/phoenix-queryserver/pull/42#discussion_r451054286
##########
File path: python-phoenixdb/phoenixdb/tests/test_sqlalchemy.py
##########
@@ -52,6 +52,39 @@ def test_textual(self):
finally:
connection.execute('drop table if exists ALCHEMY_TEST')
+ def test_schema_filtering(self):
+ engine = self._create_engine()
+ with engine.connect() as connection:
+ try:
+ connection.execute('drop table if exists ALCHEMY_TEST')
+ connection.execute('drop table if exists A.ALCHEMY_TEST_A')
+ connection.execute('drop table if exists B.ALCHEMY_TEST_B')
+
+ connection.execute(text('create table ALCHEMY_TEST (ID integer primary key)'))
+ connection.execute(text('create table A.ALCHEMY_TEST_A (ID_A integer primary key)'))
+ connection.execute(text('create table B.ALCHEMY_TEST_B (ID_B integer primary key)'))
+
+ inspector = db.inspect(engine)
+
+ self.assertEqual(inspector.get_schema_names(), [None, 'A', 'B', 'SYSTEM'])
Review comment:
Is returning `None` vs `''` (empty string) intentional? i.e. is this convention for sqlalchemy?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix-queryserver] stoty commented on a change in pull request #42: PHOENIX-5994 SqlAlchemy schema filtering incorrect semantics
Posted by GitBox <gi...@apache.org>.
stoty commented on a change in pull request #42:
URL: https://github.com/apache/phoenix-queryserver/pull/42#discussion_r453852457
##########
File path: python-phoenixdb/phoenixdb/avatica/client.py
##########
@@ -301,18 +313,26 @@ def connection_sync(self, connection_id, connProps=None):
:returns:
A ``common_pb2.ConnectionProperties`` object.
"""
- if connProps is None:
+ if not connProps:
connProps = {}
request = requests_pb2.ConnectionSyncRequest()
request.connection_id = connection_id
- request.conn_props.auto_commit = connProps.get('autoCommit', False)
request.conn_props.has_auto_commit = True
- request.conn_props.read_only = connProps.get('readOnly', False)
request.conn_props.has_read_only = True
- request.conn_props.transaction_isolation = connProps.get('transactionIsolation', 0)
- request.conn_props.catalog = connProps.get('catalog', '')
- request.conn_props.schema = connProps.get('schema', '')
+ if 'autoCommit' in connProps:
+ request.conn_props.auto_commit = connProps.pop('autoCommit')
Review comment:
That's a good idea, the caller may want to reuse a connProps instance.
##########
File path: python-phoenixdb/phoenixdb/connection.py
##########
@@ -135,50 +158,51 @@ def set_session(self, autocommit=None, readonly=None):
:param readonly:
Switch the connection to read-only mode.
"""
- props = {}
- if autocommit is not None:
- props['autoCommit'] = bool(autocommit)
- if readonly is not None:
- props['readOnly'] = bool(readonly)
- props = self._client.connection_sync(self._id, props)
- self._autocommit = props.auto_commit
- self._readonly = props.read_only
- self._transactionisolation = props.transaction_isolation
+ props = Connection._map_legacy_avatica_props(props)
+ self._avatica_props = self._client.connection_sync_dict(self._id, props)
@property
def autocommit(self):
"""Read/write attribute for switching the connection's autocommit mode."""
- return self._autocommit
+ return self._avatica_props['autoCommit']
@autocommit.setter
def autocommit(self, value):
if self._closed:
raise ProgrammingError('the connection is already closed')
- props = self._client.connection_sync(self._id, {'autoCommit': bool(value)})
- self._autocommit = props.auto_commit
+ self._avatica_props = self._client.connection_sync_dict(self._id, {'autoCommit': bool(value)})
@property
def readonly(self):
"""Read/write attribute for switching the connection's readonly mode."""
- return self._readonly
+ return self._avatica_props['readOnly']
@readonly.setter
def readonly(self, value):
if self._closed:
raise ProgrammingError('the connection is already closed')
- props = self._client.connection_sync(self._id, {'readOnly': bool(value)})
- self._readonly = props.read_only
+ self._avatica_props = self._client.connection_sync_dict(self._id, {'readOnly': bool(value)})
@property
def transactionisolation(self):
- return self._transactionisolation
+ return self._avatica_props['_transactionIsolation']
@transactionisolation.setter
def transactionisolation(self, value):
if self._closed:
raise ProgrammingError('the connection is already closed')
- props = self._client.connection_sync(self._id, {'transactionIsolation': bool(value)})
- self._transactionisolation = props.transaction_isolation
+ self._avatica_props = self._client.connection_sync_dict(self._id, {'transactionIsolation': bool(value)})
+
+ def meta(self):
+ """Creates a new meta.
+
+ :returns:
+ A :class:`~phoenixdb.meta` object.
+ """
+ if self._closed:
+ raise ProgrammingError('the connection is already closed')
Review comment:
Sure. I'll search and replace it everywhere.
##########
File path: python-phoenixdb/phoenixdb/sqlalchemy_phoenix.py
##########
@@ -126,76 +126,65 @@ def create_connect_args(self, url):
))
return [phoenix_url], connect_args
- def has_table(self, connection, table_name, schema=None):
+ def has_table(self, connection, table_name, schema=None, **kw):
if schema is None:
- query = "SELECT 1 FROM system.catalog WHERE table_name = ? LIMIT 1"
- params = [table_name.upper()]
- else:
- query = "SELECT 1 FROM system.catalog WHERE table_name = ? AND TABLE_SCHEM = ? LIMIT 1"
- params = [table_name.upper(), schema.upper()]
- return connection.execute(query, params).first() is not None
+ schema = ''
+ return bool(connection.connect().connection.meta().get_tables(
+ tableNamePattern=table_name,
+ schemaPattern=schema,
+ typeList=('TABLE', 'SYSTEM_TABLE')))
def get_schema_names(self, connection, **kw):
- query = "SELECT DISTINCT TABLE_SCHEM FROM SYSTEM.CATALOG"
- return [row[0] for row in connection.execute(query)]
+ schemas = connection.connect().connection.meta().get_schemas()
+ return [schema['TABLE_SCHEM'] for schema in schemas]
- def get_table_names(self, connection, schema=None, **kw):
+ def get_table_names(self, connection, schema=None, order_by=None, **kw):
+ '''order_by is ignored'''
if schema is None:
- query = "SELECT DISTINCT table_name FROM SYSTEM.CATALOG"
- params = []
- else:
- query = "SELECT DISTINCT table_name FROM SYSTEM.CATALOG WHERE TABLE_SCHEM = ? "
- params = [schema.upper()]
- return [row[0] for row in connection.execute(query, params)]
+ schema = ''
+ tables = connection.connect().connection.meta().get_tables(
+ schemaPattern=schema, typeList=('TABLE', 'SYSTEM_TABLE'))
+ return [table['TABLE_NAME'] for table in tables]
+
+ def get_view_names(self, connection, schema=None, **kw):
+ if schema is None:
+ schema = ''
+ return connection.connect().connection.meta().get_tables(schemaPattern=schema,
+ typeList=('VIEW'))
def get_columns(self, connection, table_name, schema=None, **kw):
if schema is None:
- query = """SELECT COLUMN_NAME, DATA_TYPE, NULLABLE
- FROM system.catalog
- WHERE table_name = ?
- AND ORDINAL_POSITION is not null
- ORDER BY ORDINAL_POSITION"""
- params = [table_name.upper()]
- else:
- query = """SELECT COLUMN_NAME, DATA_TYPE, NULLABLE
- FROM system.catalog
- WHERE TABLE_SCHEM = ?
- AND table_name = ?
- AND ORDINAL_POSITION is not null
- ORDER BY ORDINAL_POSITION"""
- params = [schema.upper(), table_name.upper()]
-
- # get all of the fields for this table
- c = connection.execute(query, params)
- cols = []
- while True:
- row = c.fetchone()
- if row is None:
- break
- name = row[0]
- col_type = COLUMN_DATA_TYPE[row[1]]
- nullable = row[2] == 1 if True else False
-
- col_d = {
- 'name': name,
- 'type': col_type,
- 'nullable': nullable,
- 'default': None
- }
-
- cols.append(col_d)
- return cols
-
- # TODO This should be possible to implement
- def get_pk_constraint(self, conn, table_name, schema=None, **kw):
+ schema = ''
+ raw = connection.connect().connection.meta().get_columns(
+ schemaPattern=schema, tableNamePattern=table_name)
+ return [self._map_column(row) for row in raw]
+
+ def get_pk_constraint(self, connection, table_name, schema=None, **kw):
+ if schema is None:
+ schema = ''
+ columns = connection.connect().connection.meta().get_columns(
+ schemaPattern=schema, tableNamePattern=table_name, *kw)
+ pk_columns = [col['COLUMN_NAME'] for col in columns if col['KEY_SEQ'] > 0]
+ return {'constrained_columns': pk_columns}
+
+ def get_indexes(self, conn, table_name, schema=None, **kw):
+ '''This information does not seem to be exposed via Avatica
+ TODO: Implement by directly querying SYSTEM tables ? '''
return []
def get_foreign_keys(self, conn, table_name, schema=None, **kw):
+ '''Foreign keys are a foreign concept to Phoenix'''
Review comment:
That doesn't work.
SqlAlchemy assumes that if we implement _get_pk_constraint()_ we also implement _get_foreign_keys()_, and fails to initialize its Table object without it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix-queryserver] joshelser commented on a change in pull request #42: PHOENIX-5994 SqlAlchemy schema filtering incorrect semantics
Posted by GitBox <gi...@apache.org>.
joshelser commented on a change in pull request #42:
URL: https://github.com/apache/phoenix-queryserver/pull/42#discussion_r453872297
##########
File path: python-phoenixdb/phoenixdb/sqlalchemy_phoenix.py
##########
@@ -126,76 +126,65 @@ def create_connect_args(self, url):
))
return [phoenix_url], connect_args
- def has_table(self, connection, table_name, schema=None):
+ def has_table(self, connection, table_name, schema=None, **kw):
if schema is None:
- query = "SELECT 1 FROM system.catalog WHERE table_name = ? LIMIT 1"
- params = [table_name.upper()]
- else:
- query = "SELECT 1 FROM system.catalog WHERE table_name = ? AND TABLE_SCHEM = ? LIMIT 1"
- params = [table_name.upper(), schema.upper()]
- return connection.execute(query, params).first() is not None
+ schema = ''
+ return bool(connection.connect().connection.meta().get_tables(
+ tableNamePattern=table_name,
+ schemaPattern=schema,
+ typeList=('TABLE', 'SYSTEM_TABLE')))
def get_schema_names(self, connection, **kw):
- query = "SELECT DISTINCT TABLE_SCHEM FROM SYSTEM.CATALOG"
- return [row[0] for row in connection.execute(query)]
+ schemas = connection.connect().connection.meta().get_schemas()
+ return [schema['TABLE_SCHEM'] for schema in schemas]
- def get_table_names(self, connection, schema=None, **kw):
+ def get_table_names(self, connection, schema=None, order_by=None, **kw):
+ '''order_by is ignored'''
if schema is None:
- query = "SELECT DISTINCT table_name FROM SYSTEM.CATALOG"
- params = []
- else:
- query = "SELECT DISTINCT table_name FROM SYSTEM.CATALOG WHERE TABLE_SCHEM = ? "
- params = [schema.upper()]
- return [row[0] for row in connection.execute(query, params)]
+ schema = ''
+ tables = connection.connect().connection.meta().get_tables(
+ schemaPattern=schema, typeList=('TABLE', 'SYSTEM_TABLE'))
+ return [table['TABLE_NAME'] for table in tables]
+
+ def get_view_names(self, connection, schema=None, **kw):
+ if schema is None:
+ schema = ''
+ return connection.connect().connection.meta().get_tables(schemaPattern=schema,
+ typeList=('VIEW'))
def get_columns(self, connection, table_name, schema=None, **kw):
if schema is None:
- query = """SELECT COLUMN_NAME, DATA_TYPE, NULLABLE
- FROM system.catalog
- WHERE table_name = ?
- AND ORDINAL_POSITION is not null
- ORDER BY ORDINAL_POSITION"""
- params = [table_name.upper()]
- else:
- query = """SELECT COLUMN_NAME, DATA_TYPE, NULLABLE
- FROM system.catalog
- WHERE TABLE_SCHEM = ?
- AND table_name = ?
- AND ORDINAL_POSITION is not null
- ORDER BY ORDINAL_POSITION"""
- params = [schema.upper(), table_name.upper()]
-
- # get all of the fields for this table
- c = connection.execute(query, params)
- cols = []
- while True:
- row = c.fetchone()
- if row is None:
- break
- name = row[0]
- col_type = COLUMN_DATA_TYPE[row[1]]
- nullable = row[2] == 1 if True else False
-
- col_d = {
- 'name': name,
- 'type': col_type,
- 'nullable': nullable,
- 'default': None
- }
-
- cols.append(col_d)
- return cols
-
- # TODO This should be possible to implement
- def get_pk_constraint(self, conn, table_name, schema=None, **kw):
+ schema = ''
+ raw = connection.connect().connection.meta().get_columns(
+ schemaPattern=schema, tableNamePattern=table_name)
+ return [self._map_column(row) for row in raw]
+
+ def get_pk_constraint(self, connection, table_name, schema=None, **kw):
+ if schema is None:
+ schema = ''
+ columns = connection.connect().connection.meta().get_columns(
+ schemaPattern=schema, tableNamePattern=table_name, *kw)
+ pk_columns = [col['COLUMN_NAME'] for col in columns if col['KEY_SEQ'] > 0]
+ return {'constrained_columns': pk_columns}
+
+ def get_indexes(self, conn, table_name, schema=None, **kw):
+ '''This information does not seem to be exposed via Avatica
+ TODO: Implement by directly querying SYSTEM tables ? '''
return []
def get_foreign_keys(self, conn, table_name, schema=None, **kw):
+ '''Foreign keys are a foreign concept to Phoenix'''
Review comment:
ah, that's fun! :P
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix-queryserver] joshelser commented on pull request #42: PHOENIX-5994 SqlAlchemy schema filtering incorrect semantics
Posted by GitBox <gi...@apache.org>.
joshelser commented on pull request #42:
URL: https://github.com/apache/phoenix-queryserver/pull/42#issuecomment-657740881
`0bafa1b` looks great. Ship it!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix-queryserver] stoty closed pull request #42: PHOENIX-5994 SqlAlchemy schema filtering incorrect semantics
Posted by GitBox <gi...@apache.org>.
stoty closed pull request #42:
URL: https://github.com/apache/phoenix-queryserver/pull/42
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix-queryserver] stoty commented on pull request #42: PHOENIX-5994 SqlAlchemy schema filtering incorrect semantics
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #42:
URL: https://github.com/apache/phoenix-queryserver/pull/42#issuecomment-657583878
I've extensively rewritten the MetaData handling (and some other components)
Please check the new code as well.
This version (hopefully) consistently uses the empty string to indicate the default schema, catalog, etc.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix-queryserver] joshelser commented on a change in pull request #42: PHOENIX-5994 SqlAlchemy schema filtering incorrect semantics
Posted by GitBox <gi...@apache.org>.
joshelser commented on a change in pull request #42:
URL: https://github.com/apache/phoenix-queryserver/pull/42#discussion_r453710649
##########
File path: python-phoenixdb/phoenixdb/avatica/client.py
##########
@@ -301,18 +313,26 @@ def connection_sync(self, connection_id, connProps=None):
:returns:
A ``common_pb2.ConnectionProperties`` object.
"""
- if connProps is None:
+ if not connProps:
connProps = {}
request = requests_pb2.ConnectionSyncRequest()
request.connection_id = connection_id
- request.conn_props.auto_commit = connProps.get('autoCommit', False)
request.conn_props.has_auto_commit = True
- request.conn_props.read_only = connProps.get('readOnly', False)
request.conn_props.has_read_only = True
- request.conn_props.transaction_isolation = connProps.get('transactionIsolation', 0)
- request.conn_props.catalog = connProps.get('catalog', '')
- request.conn_props.schema = connProps.get('schema', '')
+ if 'autoCommit' in connProps:
+ request.conn_props.auto_commit = connProps.pop('autoCommit')
Review comment:
Should we copy `connProps` in the non-None case to avoid being destructive on the `connProps` passed in?
##########
File path: python-phoenixdb/phoenixdb/connection.py
##########
@@ -135,50 +158,51 @@ def set_session(self, autocommit=None, readonly=None):
:param readonly:
Switch the connection to read-only mode.
"""
- props = {}
- if autocommit is not None:
- props['autoCommit'] = bool(autocommit)
- if readonly is not None:
- props['readOnly'] = bool(readonly)
- props = self._client.connection_sync(self._id, props)
- self._autocommit = props.auto_commit
- self._readonly = props.read_only
- self._transactionisolation = props.transaction_isolation
+ props = Connection._map_legacy_avatica_props(props)
+ self._avatica_props = self._client.connection_sync_dict(self._id, props)
@property
def autocommit(self):
"""Read/write attribute for switching the connection's autocommit mode."""
- return self._autocommit
+ return self._avatica_props['autoCommit']
@autocommit.setter
def autocommit(self, value):
if self._closed:
raise ProgrammingError('the connection is already closed')
- props = self._client.connection_sync(self._id, {'autoCommit': bool(value)})
- self._autocommit = props.auto_commit
+ self._avatica_props = self._client.connection_sync_dict(self._id, {'autoCommit': bool(value)})
@property
def readonly(self):
"""Read/write attribute for switching the connection's readonly mode."""
- return self._readonly
+ return self._avatica_props['readOnly']
@readonly.setter
def readonly(self, value):
if self._closed:
raise ProgrammingError('the connection is already closed')
- props = self._client.connection_sync(self._id, {'readOnly': bool(value)})
- self._readonly = props.read_only
+ self._avatica_props = self._client.connection_sync_dict(self._id, {'readOnly': bool(value)})
@property
def transactionisolation(self):
- return self._transactionisolation
+ return self._avatica_props['_transactionIsolation']
@transactionisolation.setter
def transactionisolation(self, value):
if self._closed:
raise ProgrammingError('the connection is already closed')
- props = self._client.connection_sync(self._id, {'transactionIsolation': bool(value)})
- self._transactionisolation = props.transaction_isolation
+ self._avatica_props = self._client.connection_sync_dict(self._id, {'transactionIsolation': bool(value)})
+
+ def meta(self):
+ """Creates a new meta.
+
+ :returns:
+ A :class:`~phoenixdb.meta` object.
+ """
+ if self._closed:
+ raise ProgrammingError('the connection is already closed')
Review comment:
nit: make it a complete sentence, `The .. already closed.`
##########
File path: python-phoenixdb/phoenixdb/sqlalchemy_phoenix.py
##########
@@ -126,76 +126,65 @@ def create_connect_args(self, url):
))
return [phoenix_url], connect_args
- def has_table(self, connection, table_name, schema=None):
+ def has_table(self, connection, table_name, schema=None, **kw):
if schema is None:
- query = "SELECT 1 FROM system.catalog WHERE table_name = ? LIMIT 1"
- params = [table_name.upper()]
- else:
- query = "SELECT 1 FROM system.catalog WHERE table_name = ? AND TABLE_SCHEM = ? LIMIT 1"
- params = [table_name.upper(), schema.upper()]
- return connection.execute(query, params).first() is not None
+ schema = ''
+ return bool(connection.connect().connection.meta().get_tables(
+ tableNamePattern=table_name,
+ schemaPattern=schema,
+ typeList=('TABLE', 'SYSTEM_TABLE')))
def get_schema_names(self, connection, **kw):
- query = "SELECT DISTINCT TABLE_SCHEM FROM SYSTEM.CATALOG"
- return [row[0] for row in connection.execute(query)]
+ schemas = connection.connect().connection.meta().get_schemas()
+ return [schema['TABLE_SCHEM'] for schema in schemas]
- def get_table_names(self, connection, schema=None, **kw):
+ def get_table_names(self, connection, schema=None, order_by=None, **kw):
+ '''order_by is ignored'''
if schema is None:
- query = "SELECT DISTINCT table_name FROM SYSTEM.CATALOG"
- params = []
- else:
- query = "SELECT DISTINCT table_name FROM SYSTEM.CATALOG WHERE TABLE_SCHEM = ? "
- params = [schema.upper()]
- return [row[0] for row in connection.execute(query, params)]
+ schema = ''
+ tables = connection.connect().connection.meta().get_tables(
+ schemaPattern=schema, typeList=('TABLE', 'SYSTEM_TABLE'))
+ return [table['TABLE_NAME'] for table in tables]
+
+ def get_view_names(self, connection, schema=None, **kw):
+ if schema is None:
+ schema = ''
+ return connection.connect().connection.meta().get_tables(schemaPattern=schema,
+ typeList=('VIEW'))
def get_columns(self, connection, table_name, schema=None, **kw):
if schema is None:
- query = """SELECT COLUMN_NAME, DATA_TYPE, NULLABLE
- FROM system.catalog
- WHERE table_name = ?
- AND ORDINAL_POSITION is not null
- ORDER BY ORDINAL_POSITION"""
- params = [table_name.upper()]
- else:
- query = """SELECT COLUMN_NAME, DATA_TYPE, NULLABLE
- FROM system.catalog
- WHERE TABLE_SCHEM = ?
- AND table_name = ?
- AND ORDINAL_POSITION is not null
- ORDER BY ORDINAL_POSITION"""
- params = [schema.upper(), table_name.upper()]
-
- # get all of the fields for this table
- c = connection.execute(query, params)
- cols = []
- while True:
- row = c.fetchone()
- if row is None:
- break
- name = row[0]
- col_type = COLUMN_DATA_TYPE[row[1]]
- nullable = row[2] == 1 if True else False
-
- col_d = {
- 'name': name,
- 'type': col_type,
- 'nullable': nullable,
- 'default': None
- }
-
- cols.append(col_d)
- return cols
-
- # TODO This should be possible to implement
- def get_pk_constraint(self, conn, table_name, schema=None, **kw):
+ schema = ''
+ raw = connection.connect().connection.meta().get_columns(
+ schemaPattern=schema, tableNamePattern=table_name)
+ return [self._map_column(row) for row in raw]
+
+ def get_pk_constraint(self, connection, table_name, schema=None, **kw):
+ if schema is None:
+ schema = ''
+ columns = connection.connect().connection.meta().get_columns(
+ schemaPattern=schema, tableNamePattern=table_name, *kw)
+ pk_columns = [col['COLUMN_NAME'] for col in columns if col['KEY_SEQ'] > 0]
+ return {'constrained_columns': pk_columns}
+
+ def get_indexes(self, conn, table_name, schema=None, **kw):
+ '''This information does not seem to be exposed via Avatica
+ TODO: Implement by directly querying SYSTEM tables ? '''
return []
def get_foreign_keys(self, conn, table_name, schema=None, **kw):
+ '''Foreign keys are a foreign concept to Phoenix'''
Review comment:
Should we throw some kind of NotImplemented Exception?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix-queryserver] joshelser commented on a change in pull request #42: PHOENIX-5994 SqlAlchemy schema filtering incorrect semantics
Posted by GitBox <gi...@apache.org>.
joshelser commented on a change in pull request #42:
URL: https://github.com/apache/phoenix-queryserver/pull/42#discussion_r451068390
##########
File path: python-phoenixdb/phoenixdb/tests/test_sqlalchemy.py
##########
@@ -52,6 +52,39 @@ def test_textual(self):
finally:
connection.execute('drop table if exists ALCHEMY_TEST')
+ def test_schema_filtering(self):
+ engine = self._create_engine()
+ with engine.connect() as connection:
+ try:
+ connection.execute('drop table if exists ALCHEMY_TEST')
+ connection.execute('drop table if exists A.ALCHEMY_TEST_A')
+ connection.execute('drop table if exists B.ALCHEMY_TEST_B')
+
+ connection.execute(text('create table ALCHEMY_TEST (ID integer primary key)'))
+ connection.execute(text('create table A.ALCHEMY_TEST_A (ID_A integer primary key)'))
+ connection.execute(text('create table B.ALCHEMY_TEST_B (ID_B integer primary key)'))
+
+ inspector = db.inspect(engine)
+
+ self.assertEqual(inspector.get_schema_names(), [None, 'A', 'B', 'SYSTEM'])
Review comment:
> The internal representation in Phoenix is null, which translates to None.
So, Phoenix is really weird when it comes to "null" and "empty string", in that they are functionally equivalent. Not sure if that's a surprise to you.
That all said, `None` is fine. Mostly curious :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix-queryserver] stoty commented on a change in pull request #42: PHOENIX-5994 SqlAlchemy schema filtering incorrect semantics
Posted by GitBox <gi...@apache.org>.
stoty commented on a change in pull request #42:
URL: https://github.com/apache/phoenix-queryserver/pull/42#discussion_r451061757
##########
File path: python-phoenixdb/phoenixdb/tests/test_sqlalchemy.py
##########
@@ -52,6 +52,39 @@ def test_textual(self):
finally:
connection.execute('drop table if exists ALCHEMY_TEST')
+ def test_schema_filtering(self):
+ engine = self._create_engine()
+ with engine.connect() as connection:
+ try:
+ connection.execute('drop table if exists ALCHEMY_TEST')
+ connection.execute('drop table if exists A.ALCHEMY_TEST_A')
+ connection.execute('drop table if exists B.ALCHEMY_TEST_B')
+
+ connection.execute(text('create table ALCHEMY_TEST (ID integer primary key)'))
+ connection.execute(text('create table A.ALCHEMY_TEST_A (ID_A integer primary key)'))
+ connection.execute(text('create table B.ALCHEMY_TEST_B (ID_B integer primary key)'))
+
+ inspector = db.inspect(engine)
+
+ self.assertEqual(inspector.get_schema_names(), [None, 'A', 'B', 'SYSTEM'])
Review comment:
I am not actually sure. The documentation doesn't specify this.
The internal representation in Phoenix is null, which translates to None.
When specifying the schema filter, the default schema is also represented in sqlAlchemy by None.
Based on this, returning None here seems to make sense.
I'll try to figure out what i.e. the mysql driver returns here, and adjust if it returns ''
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org