-
Notifications
You must be signed in to change notification settings - Fork 595
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
perf(backends): speed up most memtable existence checks #10067
perf(backends): speed up most memtable existence checks #10067
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple questions. Once #9695 lands we could also change the default implementation to
try:
self.table(name)
return True
except TableNotFound:
return False
|
faf78a6
to
89d4253
Compare
Ok, I managed to audit all the backends except for Druid and Flink. Those can be tackled in a follow-up. |
4a0537b
to
4f2ab29
Compare
@@ -411,11 +411,18 @@ def _register_udfs(self, expr: ir.Expr) -> None: | |||
self._session.udf.register(f"unwrap_json_{typ.__name__}", unwrap_json(typ)) | |||
self._session.udf.register("unwrap_json_float", unwrap_json_float) | |||
|
|||
def _in_memory_table_exists(self, name: str) -> bool: | |||
sql = f"SHOW TABLES IN {self.current_database} LIKE '{name}'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow self._session.catalog.tableExists
doesn't give the same answer as the code I have here, even when passing a fully quoted default.$memtable
(the second dbName
parameter is deprecated and the deprecation warning suggests passing the fully qualified name instead.)
4f2ab29
to
0188256
Compare
Clouds are passing:
|
0188256
to
bcbf310
Compare
Adds some slightly faster checks for memtable existence. At some point we may want to expand this to non-memtables, but for now these are only implemented for memtables.