prestodb / presto

The official home of the Presto distributed SQL query engine for big data
http://prestodb.io
Apache License 2.0
15.75k stars 5.28k forks source link

Integrate config and session property for history matching threshold #23019

Open hantangwangd opened 2 weeks ago

hantangwangd commented 2 weeks ago

Description

This PR integrate config property into session property for history-input-table-statistics-matching-threshold, so that we can get the property value uniformly through method getHistoryInputTableStatisticsMatchingThreshold() in SystemSessionProperties.

Besides, there exists a problem in the previous implementation logic when we trying to explicitly set the threshold to 0.0 through session property. By setting the value to 0.0 through session property, we intended to indicate that we will use historical statistics only when the input tables have exactly the same stats with those in the historical stats, but that's not the case in previous implement logic as we would then get the value from configuration property in this scenario, which is default not 0.0. This PR fix the problem as well.

Test Plan

Contributor checklist

Release Notes

== NO RELEASE NOTE ==
steveburnett commented 2 weeks ago

Is any documentation update needed for this?

hantangwangd commented 2 weeks ago

@steveburnett Thanks for the reminder, I have updated the description for history_input_table_statistics_matching_threshold in docs history-based-optimization.rst to amend the description of default value and the behavior when set to 0.0, please take a look. Additionally, should we open a dedicate issue for fixing the description of other session properties in history-based-optimization.rst? The description for these session properties should combine with the corresponding config properties.

cc @feilong-liu @jaystarshot

steveburnett commented 2 weeks ago

@steveburnett Thanks for the reminder, I have updated the description for history_input_table_statistics_matching_threshold in docs history-based-optimization.rst to amend the description of default value and the behavior when set to 0.0, please take a look. Additionally, should we open a dedicate issue for fixing the description of other session properties in history-based-optimization.rst? The description for these session properties should combine with the corresponding config properties.

cc @feilong-liu @jaystarshot

If you find inconsistencies or gaps in the doc for the HBO session properties, I would welcome updates. @feilong-liu @jaystarshot, what do you think?

hantangwangd commented 11 hours ago

Just to double confirm, the default behavior will not change, i.e. the default match threshold stays the same?

Yes, the default behavior will not change, the actual default value before and after this change are all 0.1.