[SPARK-19611][SQL] Introduce configurable table schema inference #16942
Conversation
- Add spark.sql.hive.schemaInferenceMode param to SQLConf - Add schemaFromTableProps field to CatalogTable (set to true when schema is successfully read from table props) - Perform schema inference in HiveMetastoreCatalog if schemaFromTableProps is false, depending on spark.sql.hive.schemaInferenceMode. - Update table metadata properties in HiveExternalCatalog.alterTable() - Add HiveSchemaInferenceSuite tests
|
Pinging participants from #16797: @gatorsmile, @viirya, @ericl, @mallman and @cloud-fan |
|
|
||
| test("Queries against case-sensitive tables with no schema in table properties should work " + | ||
| "when schema inference is enabled") { | ||
| withSQLConf("spark.sql.hive.schemaInferenceMode" -> "INFER_AND_SAVE") { |
There was a problem hiding this comment.
Will change this to reference the key via the constant in SQLConf rather than "spark.sql.hive.schemaInferenceMode".
| "NEVER_INFER (fallback to using the case-insensitive metastore schema instead of inferring).") | ||
| .stringConf | ||
| .transform(_.toUpperCase()) | ||
| .checkValues(Set("INFER_AND_SAVE", "INFER_ONLY", "NEVER_INFER")) |
There was a problem hiding this comment.
As mentioned in the PR, I'm looking for a good place to store these values as constants or an enum.
| .stringConf | ||
| .transform(_.toUpperCase()) | ||
| .checkValues(Set("INFER_AND_SAVE", "INFER_ONLY", "NEVER_INFER")) | ||
| .createWithDefault("INFER_AND_SAVE") |
There was a problem hiding this comment.
I'm open for discussion on whether or not this should be the default behavior.
|
Test build #72947 has finished for PR 16942 at commit
|
|
Tests appear to be failing due to the following error: I don't think anything in this PR should've changed the behavior of core SQL tests, but I'll look in to this. |
|
Accidentally did a force-push to my branch for this issue. Looks like I'll have to open a new PR. UPDATE: replaced by #16944 |
|
Force pushing your branch shouldn't close the PR. You didn't close it manually? |
|
@mallman If I did close it then it was by mistake. The "Reopen and comment" button was disabled with a message about the PR being closed by a force push when I hovered over it. Afraid I'm a bit of a n00b on GitHub PRs :/ |
|
Weird. I think I've seen that behavior once before. But I think the only time I force push on a PR is to rebase. Maybe that's the only kind of force push allowed for Github PRs. |
3 participants
EDIT: accidentally broke this PR with a force push. Opened #16944 to replace it.
Replaces #16797. See the discussion in this PR for more details/justification for this change.
Summary of changes
JIRA for this change
successfully read from table props)
false, depending on spark.sql.hive.schemaInferenceMode.
How was this patch tested?
The tests in HiveSchemaInferenceSuite should verify that schema inference is working as expected.
Open issues
spark.sql.hive.schemaInferenceMode(e.g. "INFER_AND_SAVE", "INFER_ONLY", "NEVER_INFER") should be made into constants or an enum. I couldn't find a sensible object to place them in though that doesn't introduce a dependency between sql/core and sql/hive.HiveExternalCatalog.alterTable()the appropriate place to write back the table metadata properties outside of createTable()? Should a new external catalog method like updateTableMetadata() be introduced?