Tests are failing relating to a feature added in PR 46089 for drivers that inherit from a driver that can limit json length.
Selecting huge json objects comes with obvious memory pressure. We want to limit that. To do this, we have a
multimethod called json-field-length that knows how to get the length of json fields.
We want to use this to select only those values which aren't "too big"™.
The default implementation returns :metabase.driver.sql/nyi (presumably not-yet-implemented).
(defmulti json-field-length
"Return a HoneySQL expression that calculates the number of characters in a JSON field for a given driver.
`json-field-identifier` is the Identifier ([[metabase.util.honey-sql-2/Identifier]]) for a JSON field."
{:added "0.49.22", :arglists '([driver json-field-identifier])}
driver/dispatch-on-initialized-driver
:hierarchy #'driver/hierarchy)
(defmethod json-field-length :default
[_driver _native-form]
;; we rely on this to tell if the method is implemented for this driver or not
::nyi)This is implemented for postgres and mysql
sql=> (methods json-field-length)
{:postgres #object[metabase.driver.postgres$eval197275$fn__197276
"0x7762a4d0"
"metabase.driver.postgres$eval197275$fn__197276@7762a4d0"],
:default #object[metabase.driver.sql$eval202869$fn__202870
"0x2c04ddb6"
"metabase.driver.sql$eval202869$fn__202870@2c04ddb6"],
:mysql #object[metabase.driver.mysql$eval146554$fn__146555
"0x6b97fe35"
"metabase.driver.mysql$eval146554$fn__146555@6b97fe35"]}The test in metabase.driver.sql-jdbc.sync.describe-table-test/long-json-sample-json-query-test has a function called
do-with-removed-method that is attempting to compare the behavior of sync with and without the benefits of the json-field-length
multimethod.
It attempts to remove the method for the current driver, test the behavior, and then reinstall that method.
"If driver.sql/json-field-length is not implemented for the driver don't omit the long value"
I saw something funky going on after running tests and seeing which methods are present in the multimethod:
(methods driver.sql/json-field-length)
{:postgres #object[metabase.driver.sql_jdbc.sync.describe_table_test$fn__185882$fn__185922$fn__185923$fn__185924$fn__185936$do_with_removed_method__185937$fn__185938
"0x398063f0"
"metabase.driver.sql_jdbc.sync.describe_table_test$fn__185882$fn__185922$fn__185923$fn__185924$fn__185936$do_with_removed_method__185937$fn__185938@398063f0"],
:default #object[metabase.driver.sql$eval142715$fn__142716
"0x27def753"
"metabase.driver.sql$eval142715$fn__142716@27def753"],
:mysql #object[metabase.driver.mysql$eval146554$fn__146555
"0x6b97fe35"
"metabase.driver.mysql$eval146554$fn__146555@6b97fe35"]}I was surprised to see one of the implementations was from the test namespace (metabase.driver.sql_jdbc.sync.describe_table_test$fn__185882...)
And that's because this is doing a bit of hacky stuff to contrast the behavior with and without this feature enabled.
But it's thinking that the driver under test directly provides an implementation. So it removes it for the current driver, runs the test then installs it for the current driver. But because of inheritance, the driver might not have an instance to begin with. Meaning it did not remove the operative multimethod, and therefore doesn't see the behavior without this feature.
If you implement it for the driver, you might still run into an issue because when it removes it for the inheriting driver, the parent driver still provides the feature. Thus again the code does not see sync without this feature.
The fix is to grab all ancestors of the current driver, intersect that with the implementing methods in the multmethod,
remove them all, run the test, and then reinstall the methods.
In pseudo code it is
(let [ancestors (conj (ancestors driver) driver)
to-remove (intersect all-methods ancestors)]
(try (doseq [method to-remove]
(uninstall multimethod method))
(run-test-without-feature)
(finally
(reinstall methods))))The patch below applies cleanly to a branch from near master.