fix: implement describe-fields to fix field sync on Metabase v0.49+#14
Open
jake-mahon-netwrix wants to merge 4 commits into
Open
fix: implement describe-fields to fix field sync on Metabase v0.49+#14jake-mahon-netwrix wants to merge 4 commits into
jake-mahon-netwrix wants to merge 4 commits into
Conversation
DatabaseMetaData.getColumns() via JDBC returns no rows against Databend, causing Metabase to store zero field metadata for all tables. MBQL queries then fail with "Generated program is invalid" because there are no field IDs. Replaces the broken JDBC metadata path with a direct SQL query to information_schema.columns, which returns complete column metadata correctly. Removes duplicate database-type->base-type defmethod. Also upgrades databend-jdbc from 0.4.2 to 0.4.6, pins Clojure to 1.11.1, and changes :aot :all to :aot [metabase.driver.databend] to avoid compiling provided Metabase core namespaces at build time. Generated with AI Co-Authored-By: Claude Code <ai@netwrix.com>
…a_type - Remove the simple database-type->base-type defmethod that shadowed the Nullable-stripping one, restoring correct type mapping for JDBC paths - Guard data_type nil in describe-table-fields-via-sql to prevent NPE on virtual/computed columns and skip rows with empty type Generated with AI Co-Authored-By: Claude Code <ai@netwrix.com>
DatabaseMetaData.getColumns() returns no rows against Databend, so Metabase stores zero field metadata causing all MBQL queries to fail. Two fixes: 1. Add driver/describe-fields :databend using jdbc/query against information_schema.columns (faster path used by Metabase v0.49+). Returns :table-schema nil to match Postgres schema=NULL stored by describe-database (JDBC getTables() returns TABLE_SCHEM=""). 2. Update describe-table to use the same direct SQL path as fallback. Also upgrades databend-jdbc from 0.4.2 to 0.4.6 and declares :describe-fields support explicitly in database-supports?. Generated with AI Co-Authored-By: Claude Code <ai@netwrix.com>
Covers early exit on empty schema/table-names, db-name fallback logic, :table-schema nil invariant, type filtering (AggregateFunction and nil), IN-clause parameterization, database-supports? registration, and the exception-handling path. Also fixes the test alias (was missing +provided and the task name) and removes four stale cljc.java-time requires that blocked test loading. Integration tests available via `lein test-integration`. Generated with AI Co-Authored-By: Claude Code <ai@netwrix.com>
Author
|
This PR was made using sonnet 4.6 with opus 4.8 plan review and code review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On Metabase v0.49.1+, field sync calls
driver/describe-fields(notdriver/describe-table). Since:databendinherits from:sql-jdbc, it used the default:sql-jdbcimplementation, which callsDatabaseMetaData.getColumns(). Against Databend,getColumns()returns no rows — leaving every table with zero fields and breaking all MBQL queries.Root causes (three separate issues)
1. Wrong method:
sync-fields!callsdescribe-fields, notdescribe-table. The:databenddriver needs its owndescribe-fieldsdefmethod, and:describe-fields truemust be added todatabase-supports?.2.
do-with-connection-with-optionsreturns 0 rows: The sameinformation_schema.columnsSQL that returns 850 rows viaclojure.java.jdbc/queryreturns 0 rows when executed throughdo-with-connection-with-options+ PreparedStatement against the Databend JDBC driver. Root cause is unclear (likely a Databend JDBC driver behavior);jdbc/query(same pattern used bydb-default-timezone) works correctly.3.
:table-schema nil, not"": Databend's JDBCgetTables()returnsTABLE_SCHEM="", causing Metabase to storeschema = NULLin its Postgres app DB. Metabase's sync code matches fields to tables via(some-> table-schema lower-case-en)—nilgeneratesWHERE schema IS NULL(correct), but""generatesWHERE lower(schema) = ''(finds nothing).Fix
(defmethod driver/describe-fields :databend ...)that queriesinformation_schema.columnsdirectly viajdbc/query.:describe-fields trueto thedatabase-supports?doseq.:table-schema nilin every field map.Tests
New standalone unit test file (
test/metabase/driver/databend_describe_fields_test.clj) — runs without a live Databend server. Covers:db-namefallback (nil schema-names →:details :dbname→"default"):table-schema nildata_typehandlingtable-namesis provideddatabase-supports? :describe-fieldsreturns true[]without rethrowingAlso fixes the test alias in
project.clj(was["with-profile" "test"]— missing+providedand the task name).Verified
Deployed to Metabase v0.61.3.5. 850 fields across 56 tables auto-populate on every sync. MBQL queries via MCP return real data.
Design notes
schema-namesuses the first non-blank value. Databend is a single-schema warehouse; multi-schema setups are unsupported by the existing driver anyway.catchreturns[]rather than rethrowing, keeping sync from failing entirely on a transient error. Happy to change this to propagate if the project prefers that behavior.