Skip to content

Python: Promote ClickHouse SQL models - #5950

Merged
yoff merged 6 commits into
github:mainfrom
RasmusWL:promote-clickhouse
Jun 16, 2021
Merged

Python: Promote ClickHouse SQL models#5950
yoff merged 6 commits into
github:mainfrom
RasmusWL:promote-clickhouse

Conversation

@RasmusWL

Copy link
Copy Markdown
Member

No description provided.

RasmusWL added 4 commits May 25, 2021 14:27
This did reveal a few places where we do not detect the incoming SQL
This did turn into a few changes, that maybe could have been split into
separate PRs 🤷

* Rename `ClickHouseDriver` => `ClickhouseDriver`, to better follow
  import name in `.qll` name
* Rewrote modeling to use API graphs
* Split modeling of `aioch` into separate `.qll` file, which does re-use
  the `getExecuteMethodName` predicate. I feel that sharing code between
  the modeling like this was the best approach, and stuck the
  `INTERNAL: Do not use.` labels on both modules.
* I also added handling of keyword arguments (see change in .py files)
@RasmusWL

Copy link
Copy Markdown
Member Author

seems like stripping leading whitespace sneaked into my merge commit... I don't feel too sorry about that 😄

@yoff yoff left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I think splitting out aioch works well.

}

/** `clickhouse_driver.Client` execute method names */
string getExecuteMethodName() { result in ["execute_with_progress", "execute", "execute_iter"] }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very mildly annoyed with the ordering here.. :-)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR welcome 😊

@@ -0,0 +1,2 @@
import python
import experimental.meta.ConceptsTest

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should also promote this at some point..

@yoff
yoff merged commit 0ddeb7a into github:main Jun 16, 2021
@RasmusWL
RasmusWL deleted the promote-clickhouse branch June 17, 2021 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants