rapidgithub opened a new pull request, #6952:
URL: https://github.com/apache/hive/pull/6952
### What changes were proposed in this pull request?
This PR fixes an issue where vectorized execution of `LAG` and `LEAD` window
functions returns `0.0` (or `0`) instead of the specified default literal value
when a `DOUBLE` or `DECIMAL` literal default is provided.
Specifically:
1. In `VectorPTFEvaluatorAbstractLeadLag.initDefaultValue()`:
- Handled null literal defaults (`cve.getIsNullValue()`) so
`defaultValue` properly evaluates to `null`.
- Replaced the hardcoded call to
`ConstantVectorExpression.getLongValue()` with `cve.getValue()`, reading the
literal according to its actual type.
- Added type conversion based on the evaluator's target `type`:
- **`Type.DOUBLE`**: Converts `Number` (`doubleValue()`),
`HiveDecimal`, `HiveDecimalWritable`, or String.
- **`Type.DECIMAL`**: Produces a `HiveDecimalWritable` via
`setFromDouble()`, `setFromLong()`, `HiveDecimal`, or String.
- **`Type.LONG`**: Converts `Number` (`longValue()`), `HiveDecimal`,
`HiveDecimalWritable`, or String.
- Fixed exception message string from `"for lag function"` to `"for
lead/lag function"` (as both `LAG` and `LEAD` share this base evaluator).
2. Added unit tests in `TestVectorPTFEvaluatorLeadLag.java` verifying
`DOUBLE`, `DECIMAL`, `LONG`, and `NULL` defaults for both
`VectorPTFEvaluatorLag` and `VectorPTFEvaluatorLead`.
3. Extended `vector_ptf_lead_lag.q` to test literal defaults with both
vectorized execution disabled and enabled.
### Why are the changes needed?
In `VectorPTFEvaluatorAbstractLeadLag.initDefaultValue()`, the constant
default value expression was previously initialized using:
```java
// FIXME: always getLongValue()? check if other type is given in 3rd argument
long longValue = ((ConstantVectorExpression)
defaultValueExpression).getLongValue();
```
In `ConstantVectorExpression`, constants are stored by type: double/float
literals are stored in `doubleValue`, and decimal literals are stored in
`decimalValue` (while `longValue` remains `0`).
Because `getLongValue()` was always read:
- For `Type.DOUBLE`, `Double.valueOf(0)` produced `0.0`.
- For `Type.DECIMAL`, `new HiveDecimalWritable(0)` produced `0`.
- For `NULL` constant literals, `longValue` was also read, returning `0.0` /
`0` instead of `null`.
This caused a divergence between non-vectorized and vectorized execution:
non-vectorized execution returned the expected literal default (e.g. `2.5`),
while vectorized execution returned `0.0` or `0`.
### Does this PR introduce _any_ user-facing change?
Yes. Fixes incorrect query results when vectorized execution is enabled for
queries using `LAG(...)` or `LEAD(...)` with a non-integer literal default.
#### Repro Example:
```sql
CREATE TABLE lag_default_repro (
region STRING,
sale_id INT,
amount DOUBLE
);
INSERT INTO lag_default_repro VALUES
('east', 1, 10.0),
('east', 2, 20.0),
('west', 1, 100.0);
SELECT
region, sale_id, amount,
LAG(amount, 1, 2.5) OVER (PARTITION BY region ORDER BY sale_id) AS
prev_amount_or_default
FROM lag_default_repro
ORDER BY region, sale_id;
```
- **Previous behavior (vectorized)**: Returned `0.0` for `east/1` and
`west/1`.
- **New behavior (vectorized)**: Returns `2.5` for `east/1` and `west/1`
(matching non-vectorized execution).
### How was this patch tested?
1. **Unit tests added in `TestVectorPTFEvaluatorLeadLag.java`**:
- `testLagDoubleLiteralDefault`: validates `LAG` with double literal
default (e.g. `2.5`).
- `testLeadDoubleLiteralDefault`: validates `LEAD` with double literal
default.
- `testLagDecimalLiteralDefault`: validates `LAG` with decimal literal
default.
- `testLeadDecimalLiteralDefault`: validates `LEAD` with decimal literal
default.
- `testLagDecimalFromDoubleDefault`: validates double literal default on
a decimal column.
- `testLagLongLiteralDefault`: validates long literal default.
- `testLagNullConstantDefault`: validates null constant default.
- `testLagNullExpressionDefault`: validates null expression default.
Command:
```bash
mvn test -pl ql -Dtest=TestVectorPTFEvaluatorLeadLag
```
2. **Integration Q-test**:
- Added test queries in
`ql/src/test/queries/clientpositive/vector_ptf_lead_lag.q` comparing
non-vectorized and vectorized results for `vector_ptf_lead_lag_double` and
`vector_ptf_lead_lag_decimal`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
