Sqlalchemy dev POC #30
Conversation
-a bare-bone dialect styled after PyHive and Impyla -includes minimal/stubs for DatabricksIdentifierPreparer, DatabricksExecutionContext, DatabricksTypeCompiler, & DatabricksCompiler
-bare-bone implementation of dialect -basic test suite -sample app (not working)
(secret revoked)
-clean up Makefile for pytest -explore passing in --requirements to help with suite run -exploration of DDLCompiler
Signed-off-by: George Chow <[email protected]>
Signed-off-by: George Chow <[email protected]>
Signed-off-by: George Chow <[email protected]>
Signed-off-by: George Chow <[email protected]>
overcoil
requested review from
susodapop,
arikfr,
moderakh and
yunbodeng-db
as code owners
| id: setup-python | ||
| uses: actions/setup-python@v2 | ||
| with: | ||
| python-version: 3.7 |
There was a problem hiding this comment.
does this work on other versions of python3 as well? any limitation on the supported python version?
There was a problem hiding this comment.
It should. Note that I cribbed from code-quality-checks.yml for this action to test out some ideas. Now that I think about it, this action won't be operative since I've since removed the Pytest test. I will remove this action completely.
(The current plan is to not turn on Github Action in the OSS repo; contributors are encouraged to do so and to use these though.)
| DATABRICKS_SCHEMA: ${{ secrets.REPOSEC_DATABRICKS_SCHEMA }} | ||
|
|
||
| run: | | ||
| echo y | dbsqlcli --hostname $DATABRICKS_SERVER_HOSTNAME --http-path $DATABRICKS_HTTP_PATH --access-token $DATABRICKS_TOKEN -e "USE $DATABRICKS_SCHEMA; DROP TABLE IF EXISTS t; DROP TABLE IF EXISTS tabletest; DROP TABLE IF EXISTS integer_table;" |
There was a problem hiding this comment.
what happens if there are concurrent PRs? are they going to use the same database name and table name and hence one PR CI may fail because of other PR CI writing to the same table?
There was a problem hiding this comment.
Refer to my answers to your other comment on this action.
| @@ -1,18 +0,0 @@ | |||
| # Following official SQLAlchemy guide: | |||
There was a problem hiding this comment.
where did this class come from? did we already have some sql-alchemy related code in the main branch?
There was a problem hiding this comment.
This was created initially from the rough scaffolding that Jesse and I put together earlier on (commit 0378460). I ended up tossing it after I figured out that I won't be using SA's test suite.
| if type_.precision is None: | ||
| return "DECIMAL" | ||
| elif type_.scale is None: | ||
| return "DECIMAL(%(precision)s)" % {"precision": type_.precision} |
There was a problem hiding this comment.
do we have test covering the methods of this class?
There was a problem hiding this comment.
Good catch. I will revisit integration.py to include both the default and specified scales.
| supports_unicode_statements = True | ||
| supports_unicode_binds = True | ||
| returns_unicode_strings = True | ||
| description_encoding = None |
There was a problem hiding this comment.
what's description_encoding?
There was a problem hiding this comment.
According to what I found in SA's doc (https://github.com/sqlalchemy/sqlalchemy/blob/main/doc/build/changelog/changelog_05.rst), it "is used for encoding the column name when processing the metadata. This usually defaults to utf-8." I went with "None" as for all the other dialects I studied.
|
|
||
| class DatabricksDialect(default.DefaultDialect): |
There was a problem hiding this comment.
seems src/databricks/sqlalchemy/dialect.py already existed before. Where did this come from?
did we have some sort of support for sqlalchemy dialect previously?
There was a problem hiding this comment.
The pre-existing src/databricks/sqlalchemy/dialect.py was a rough scaffolding that Jesse and I put together earlier on (see commit 0378460). I built on this scaffolding.
… specified scales Signed-off-by: George Chow <[email protected]>
|
Closing in favour of #57 although I will incorporate many of these tests into that PR. |
What type of PR is this?
Description
This is a barebone dialect together with an expanded integration test that builds on the initial scaffolding. The main add are primary/foreign key support (to ignore them if they're specified) and explicit support for the numeric, string and date/time types.
How is this tested?
I expanded integration.py to cover the scenario of creating and dropping a table with the types.
Related Tickets & Documents
PECO-174