HIVE-24203: Implement stats annotation rule for the LateralViewJoinOperator by okumin · Pull Request #1531 · apache/hive

okumin · 2020-09-28T13:45:56Z

What changes were proposed in this pull request?

Estimate statistics of LATERAL VIEW correctly.

StatsRulesProcFactory doesn't have any rules to handle a JOIN by LATERAL VIEW.
This can cause an underestimation in case that UDTF in LATERAL VIEW generates multiple rows.

Why are the changes needed?

Significant underestimation can happen when LATERAL VIEW increases the number of records a lot and the source table has large.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Added one test case.

okumin · 2020-09-29T06:15:25Z

@@ -0,0 +1,38 @@
+set hive.fetch.task.conversion=none;


To make EXPLAIN show Statistics. I'm thinking to create another ticket and add this line to other annotate_stats_*.q.

e.g. https://github.com/apache/hive/blob/master/ql/src/test/results/clientpositive/llap/annotate_stats_select.q.out

zabetak · 2020-09-29T21:05:57Z

  }

+  /**
+   * LateralViewJoinOperator joins the output of select with the output of UDTF.


Could you provide a bit more details about what the rule does? Most of the other rules in this class give a general overview of the cost model they implement.

@zabetak Thanks for taking a look!
I added a description. Please feel free to ask me if something doesn't make sense.

…erator

jcamachor · 2020-10-01T00:01:21Z

+   *
+   * For each row of the source, the left branch just picks columns and the right branch processes UDTF.
+   * And then LVJ joins a row from the left branch with rows from the right branch.
+   * The join has one-to-many relationship since UDTF can generate multiple rows.


Just leaving a note. I took a quick look at the UDTF logic and it seems the selectivity is hardcoded via config. It seems the outer flag is not taken into account either, which could be a straightforward improvement for the estimates, i.e., UDFT will produce at least as many rows as it receives.

jcamachor · 2020-10-01T00:08:47Z

+        return null;
+      }
+
+      final Statistics selectStats = parents.get(LateralViewJoinOperator.SELECT_TAG).getStatistics().clone();


Do you need to clone them? Are you modifying them? (Same for next line)

As for udtfStats, we can totally avoid clone.
As for udtfStats, its column stats will be updated. However, looks like StatsUtils.getColStatisticsFromExprMap clones them?
Anyway I think we can remove them if CI passes. I will try it.

Nothing was not unexpectedly broken. CI failed but it would not be related to this PR...

91e492d

okumin · 2020-10-06T16:06:37Z

                            Lateral View Join Operator
                              outputColumnNames: _col0, _col1, _col5, _col6
-                              Statistics: Num rows: 0 Data size: 24 Basic stats: PARTIAL Column stats: NONE
+                              Statistics: Num rows: 0 Data size: 24 Basic stats: PARTIAL Column stats: COMPLETE


With clone, the following condition is not satisfied since the basic stats of parent operators are PARTIAL.

hive/ql/src/java/org/apache/hadoop/hive/ql/optimizer/stats/annotation/StatsRulesProcFactory.java

Line 2969 in 91e492d

if (satisfyPrecondition(selectStats) && satisfyPrecondition(udtfStats)) {

BTW, it would be better that the UDTF rule puts one in num rows in case that it becomes zero.

definetly - I don't think it will be 0 in reality!

This is an edge case since HIVE_STATS_UDTF_FACTOR is greater than or equal to 1. Anyway, I created a ticket.
https://issues.apache.org/jira/browse/HIVE-24240

kgyrtkirk · 2020-10-07T12:36:40Z

+      final Statistics selectStats = parents.get(LateralViewJoinOperator.SELECT_TAG).getStatistics();
+      final Statistics udtfStats = parents.get(LateralViewJoinOperator.UDTF_TAG).getStatistics();
+
+      final double factor = (double) udtfStats.getNumRows() / (double) selectStats.getNumRows();


I know selectStats.getNumRows() should not be zero - but just in case... could you also add the resulting logic as StatsUtils or something like that?

Added steps to check both numbers and ensure at least one record on stats.
5039634

kgyrtkirk · 2020-10-07T12:44:26Z

+        joinedStats = applyRuntimeStats(aspCtx.getParseContext().getContext(), joinedStats, lop);
+        lop.setStatistics(joinedStats);
+
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("[0] STATS-" + lop.toString() + ": " + joinedStats.extendedToString());
+        }


this seems to be a common expression in both branches of the if - could you move it outside?

I wonder if we should switch [0] or [1] based on a condition. I can see some rules use a different marker based on maybe the existence of column stats.

I don't know what's the point of these [0]/[1] markers; from one of the historical commits it seems to me like these are some kind of "log message indexes" inside the method ....
I think we could stop doing that...

I also agree and I did that.
d333d5d

kgyrtkirk · 2020-10-07T13:41:27Z

+        colStats.setNumNulls(StatsUtils.safeMult(colStats.getNumNulls(), factor));
+        // When factor > 1, the same records are duplicated and countDistinct never changes.
+        if (factor < 1.0) {
+          colStats.setCountDistint(StatsUtils.safeMult(colStats.getCountDistint(), factor));


I think we should make sure that NDV is at least 1 in case numrows is >0

Now I think this is available for this purpose if we add updating num trues and num falses?

hive/ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java

Lines 2050 to 2100 in c082a72

public static void updateStats(Statistics stats, long newNumRows,

boolean useColStats, Operator<? extends OperatorDesc> op,

Set<String> affectedColumns) {

if (newNumRows < 0) {

LOG.debug("STATS-" + op.toString() + ": Overflow in number of rows. "

+ newNumRows + " rows will be set to Long.MAX_VALUE");

newNumRows = StatsUtils.getMaxIfOverflow(newNumRows);

}

if (newNumRows == 0) {

LOG.debug("STATS-" + op.toString() + ": Equals 0 in number of rows. "

+ newNumRows + " rows will be set to 1");

newNumRows = 1;

}

long oldRowCount = stats.getNumRows();

double ratio = (double) newNumRows / (double) oldRowCount;

stats.setNumRows(newNumRows);

if (useColStats) {

List<ColStatistics> colStats = stats.getColumnStats();

for (ColStatistics cs : colStats) {

long oldDV = cs.getCountDistint();

if (affectedColumns.contains(cs.getColumnName())) {

long newDV = oldDV;

// if ratio is greater than 1, then number of rows increases. This can happen

// when some operators like GROUPBY duplicates the input rows in which case

// number of distincts should not change. Update the distinct count only when

// the output number of rows is less than input number of rows.

if (ratio <= 1.0) {

newDV = (long) Math.ceil(ratio * oldDV);

}

cs.setCountDistint(newDV);

cs.setFilterColumn();

oldDV = newDV;

}

if (oldDV > newNumRows) {

cs.setCountDistint(newNumRows);

}

long newNumNulls = Math.round(ratio * cs.getNumNulls());

cs.setNumNulls(newNumNulls > newNumRows ? newNumRows: newNumNulls);

}

stats.setColumnStats(colStats);

long newDataSize = StatsUtils.getDataSizeFromColumnStats(newNumRows, colStats);

stats.setDataSize(StatsUtils.getMaxIfOverflow(newDataSize));

} else {

long newDataSize = (long) (ratio * stats.getDataSize());

stats.setDataSize(StatsUtils.getMaxIfOverflow(newDataSize));

}

}

This method may include additional logging and logics to optimize JOIN such as cs.setFilterColumn. It would be better to implement a simple and separate utility.

Ceiled. I moved this method since I'd like to reuse it for HIVE-24240.
5039634

kgyrtkirk · 2020-10-07T13:44:29Z

                            Lateral View Join Operator
                              outputColumnNames: _col0, _col1, _col5, _col6
-                              Statistics: Num rows: 0 Data size: 24 Basic stats: PARTIAL Column stats: NONE
+                              Statistics: Num rows: 0 Data size: 24 Basic stats: PARTIAL Column stats: COMPLETE


definetly - I don't think it will be 0 in reality!

okumin · 2020-10-14T06:05:56Z


-      final double factor = (double) udtfStats.getNumRows() / (double) selectStats.getNumRows();
+      final long udtfNumRows = Math.max(udtfStats.getNumRows(), 1);
+      final double factor = (double) udtfNumRows / (double) Math.max(selectStats.getNumRows(), 1);


factor will be greater than 0.0 and must not 0 or infinity.

okumin · 2020-10-14T06:07:18Z

@kgyrtkirk I have updated some points so that # of rows will never be 0.
Could you please have a look when you have a chance?

github-actions · 2020-12-14T00:53:00Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.

kgyrtkirk added tests pending tests unstable and removed tests pending labels Sep 28, 2020

okumin force-pushed the HIVE-24203 branch from 7dac2f8 to c88200f Compare September 29, 2020 04:01

kgyrtkirk added tests pending tests passed and removed tests unstable tests pending labels Sep 29, 2020

okumin commented Sep 29, 2020

View reviewed changes

zabetak reviewed Sep 29, 2020

View reviewed changes

okumin force-pushed the HIVE-24203 branch from c88200f to 01641e9 Compare September 30, 2020 03:13

kgyrtkirk added tests pending tests passed and removed tests passed tests pending labels Sep 30, 2020

okumin force-pushed the HIVE-24203 branch 2 times, most recently from b37a00a to e2479f8 Compare September 30, 2020 07:35

kgyrtkirk added tests pending tests failed and removed tests passed tests pending labels Sep 30, 2020

HIVE-24203: Implement stats annotation rule for the LateralViewJoinOp…

cc942b6

…erator

okumin force-pushed the HIVE-24203 branch from e2479f8 to cc942b6 Compare September 30, 2020 12:43

kgyrtkirk added tests pending tests passed and removed tests failed tests pending labels Sep 30, 2020

jcamachor reviewed Oct 6, 2020

View reviewed changes

Remove unnecessary clone

91e492d

kgyrtkirk removed the tests passed label Oct 6, 2020

kgyrtkirk added tests unstable and removed tests pending labels Oct 6, 2020

okumin commented Oct 6, 2020

View reviewed changes

kgyrtkirk reviewed Oct 7, 2020

View reviewed changes

okumin added 2 commits October 8, 2020 11:54

Move some steps outside

d333d5d

Ensure that num rows >= 1

5039634

kgyrtkirk added tests pending tests passed and removed tests unstable tests pending labels Oct 8, 2020

okumin commented Oct 14, 2020

View reviewed changes

github-actions Bot added the stale label Dec 14, 2020

kgyrtkirk approved these changes Dec 14, 2020

View reviewed changes

kgyrtkirk added tests pending tests passed and removed tests passed tests pending labels Dec 14, 2020

github-actions Bot closed this Dec 22, 2020

kgyrtkirk reopened this Jan 4, 2021

kgyrtkirk added tests pending tests passed and removed tests passed stale tests pending labels Jan 4, 2021

kgyrtkirk merged commit 6deef2c into apache:master Jan 13, 2021

okumin deleted the HIVE-24203 branch January 14, 2021 05:28

okumin mentioned this pull request Feb 16, 2021

HIVE-24240: Implement missing features in UDTFStatsRule #1984

Closed

okumin mentioned this pull request May 24, 2023

HIVE-24240: Implement missing features in UDTFStatsRule #4353

Merged

	public static void updateStats(Statistics stats, long newNumRows,
	boolean useColStats, Operator<? extends OperatorDesc> op,
	Set<String> affectedColumns) {

	if (newNumRows < 0) {
	LOG.debug("STATS-" + op.toString() + ": Overflow in number of rows. "
	+ newNumRows + " rows will be set to Long.MAX_VALUE");
	newNumRows = StatsUtils.getMaxIfOverflow(newNumRows);
	}
	if (newNumRows == 0) {
	LOG.debug("STATS-" + op.toString() + ": Equals 0 in number of rows. "
	+ newNumRows + " rows will be set to 1");
	newNumRows = 1;
	}

	long oldRowCount = stats.getNumRows();
	double ratio = (double) newNumRows / (double) oldRowCount;
	stats.setNumRows(newNumRows);

	if (useColStats) {
	List<ColStatistics> colStats = stats.getColumnStats();
	for (ColStatistics cs : colStats) {
	long oldDV = cs.getCountDistint();
	if (affectedColumns.contains(cs.getColumnName())) {
	long newDV = oldDV;

	// if ratio is greater than 1, then number of rows increases. This can happen
	// when some operators like GROUPBY duplicates the input rows in which case
	// number of distincts should not change. Update the distinct count only when
	// the output number of rows is less than input number of rows.
	if (ratio <= 1.0) {
	newDV = (long) Math.ceil(ratio * oldDV);
	}
	cs.setCountDistint(newDV);
	cs.setFilterColumn();
	oldDV = newDV;
	}
	if (oldDV > newNumRows) {
	cs.setCountDistint(newNumRows);
	}
	long newNumNulls = Math.round(ratio * cs.getNumNulls());
	cs.setNumNulls(newNumNulls > newNumRows ? newNumRows: newNumNulls);
	}
	stats.setColumnStats(colStats);
	long newDataSize = StatsUtils.getDataSizeFromColumnStats(newNumRows, colStats);
	stats.setDataSize(StatsUtils.getMaxIfOverflow(newDataSize));
	} else {
	long newDataSize = (long) (ratio * stats.getDataSize());
	stats.setDataSize(StatsUtils.getMaxIfOverflow(newDataSize));
	}
	}

Conversation

okumin commented Sep 28, 2020

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

okumin commented Oct 14, 2020

Uh oh!

github-actions Bot commented Dec 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants