-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Document table statistics property for Hive connector #10525
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Break up into separate PRs please as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs sorting out
d203788
to
99b444a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Please update the title of the PR to match the commit message as discussed.
Also confirm that this builds locally to verify that the GHA failure is a false alarm.
@@ -388,6 +388,11 @@ Property Name Description | |||
``hive.query-partition-filter-required`` Set to ``true`` to force a query to use a partition filter. ``false`` | |||
You can use the ``query_partition_filter_required`` catalog | |||
session property for temporary, catalog specific use. | |||
|
|||
``hive.statistics_enabled`` Disable :doc:`/optimizer/statistics`, and therefore ``true`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is confusing that the name of this is *enabled, but the description starts out with "Disable..." but the default is true.
99b444a
to
788633b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good % nit.
Set to ``false`` to disable statistics, this also disables | ||
:doc:`/optimizer/cost-based-optimizations`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpicking - this is technically true but a side effect of the fact that stats become missing. i.e. there might be future cost-based optimisations which might not be dependent on stats (or we could generate some rough stats during runtime) and then this statement would be false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could do a minor edit to say this or something similar:
Set to
false
to disable statistics. Disabling statistics
means that :doc:/optimizer/cost-based-optimizations
can
not make smart decisions about the query plan.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 on this minor edit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented edit.
788633b
to
0e89a54
Compare
No description provided.