Sync index info #36462
Sync index info #36462
Conversation
|
No failed tests 🎉 |
src/metabase/driver.clj
Outdated
| :native-requires-specified-collection}) | ||
| :native-requires-specified-collection | ||
|
|
||
| ;; Does the driver support column(s) indexing |
There was a problem hiding this comment.
I wonder if this should read:
Does the driver support column(s) support storing index info
Implementing the multimethods to sync index info should be separate from whether indices are supported on the database or not.
Also wondering whether the feature should be called :index-info
There was a problem hiding this comment.
yea index-info sounds better to me, renamed in 07847b5
| :removed-indexes 0}) | ||
|
|
||
| (defn maybe-sync-indexes-for-table! | ||
| "Sync the indexes for `table` if the driver supports indexing." |
There was a problem hiding this comment.
I think it's worth being precise here about what we mean by "supports indexing"
| "Sync the indexes for `table` if the driver supports indexing." | |
| "Sync the indexes for `table` if the driver supports storing index info." |
test/metabase/test/data/sql/ddl.clj
Outdated
| (when indexed? | ||
| (add! (sql.tx/create-index-sql driver (:table-name tabledef) [(:field-name fielddef)])))) |
There was a problem hiding this comment.
Adding this to the FK doseq block is inconsistent with the way column comments are added on lines 64-68 below, which has its own separate doseq. I don't care either way, but as long as its consistent it'll be easier to read and understand
|
@qnkhuat Did you forget to add a milestone to the issue for this PR? When and where should I add a milestone? |
Milestone 1 of #36452.
This adds a new column
metabase_field.database_indexed, it's set to true during sync if there is a single index on the column, or it's the 1st column of a composite index.For DBs that do not support indexing,
metabase_field.database_indexedshould be nil.