feat: get_objects with single query#87
Conversation
Mandukhai-Alimaa
left a comment
There was a problem hiding this comment.
Can we add a test that verifies GetObjects can actually discover session-scoped objects like temp tables? Maybe something that creates a temp table and verifies GetObjects can find it.
|
Added the test, just note that this is gonna pass only for mariadb. If we ever decide to add mysql test run in ci as well, we should somehow disable it. |
| @@ -38,7 +38,7 @@ const ( | |||
| // GetCurrentCatalog implements driverbase.CurrentNamespacer. | |||
| func (c *mysqlConnectionImpl) GetCurrentCatalog() (string, error) { | |||
There was a problem hiding this comment.
Also, we should call c.ClearPending() at the start of these connection-level SQL methods (GetCurrentCatalog, SetCurrentCatalog, PrepareDriverInfo, GetTableSchema, GetObjects), since they can run while another reader is still active on the same session and hit busy-connection / out-of-sync errors. That is already the pattern sqlwrapper uses for statement execution before reusing the dedicated connection. See adbc-drivers/driverbase-go#56
go/mysql_test.go
Outdated
| suite.Run(t, &MySQLTests{Quirks: quirks}) | ||
| } | ||
|
|
||
| func (s *MySQLTestSuite) TestGetObjectsTempTable() { |
There was a problem hiding this comment.
The CI runs tests against MySQL and the tests still seem to be passing even though you mentioned they would only pass on MariaDB. It seems the test does not actually validate the existence of the temp table, but validates the GetObjects does not error and return some data.
This happens on MySQL even when the temp table isn't found (because temp tables are not in INFORMATION_SCHEMA.TABLES), as the LEFT JOIN still returns catalog rows with empty table lists.
There was a problem hiding this comment.
my bad, I messed up the test and thought it was mariadb in compose when it was green. Should I drop the test? I don't really see a good way of testing this.
There was a problem hiding this comment.
yea i think you are right. We can drop the test.
I opted for pushing depth/filter complexity down to the sql query rather than trying to handle all the different types of results on go side. This is the query that's always run on mysql side:
Additional
1=0andLIKEjoin predicates are used to handle requested depth/filters. please feel free to disagree on the approach 😄.