Skip to content

Conversation

@andygrove
Copy link
Member

Which issue does this PR close?

N/A

Rationale for this change

The main motivation was to move the very detailed description of the different Parquet scan implementations out of the user guide and into a dedicated page in the contributor guide. Users should not need to know these details.

I also made some other minor updates.

What changes are included in this PR?

How are these changes tested?

Comment on lines -112 to -115
## Array Expressions
Expressions that are not 100% Spark-compatible will fall back to Spark by default and can be enabled by setting
`spark.comet.expression.EXPRNAME.allowIncompatible=true`, where `EXPRNAME` is the Spark expression class name. See
the [Comet Supported Expressions Guide](expressions.md) for more information on this configuration setting.

Comet has experimental support for a number of array expressions. These are experimental and currently marked
as incompatible and can be enabled by setting `spark.comet.expression.allowIncompatible=true`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous section explains incompatible expressions. There is no need to say anything special about array expressions.

Comment on lines -104 to -105
There is a known bug with using count(distinct) within aggregate queries, where each NaN value will be counted
separately [#1824](https://github.com/apache/datafusion-comet/issues/1824).
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bug is fixed

Copy link
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @andygrove
I would also include @parthchandra flow-class-landscape diagram how everything is interconnected https://docs.google.com/drawings/d/1yYyyGVlItfJ5G1m-3eYJYeg6p4Jce3TRxY83MQOrV74

@andygrove
Copy link
Member Author

Thanks @andygrove I would also include @parthchandra flow-class-landscape diagram how everything is interconnected https://docs.google.com/drawings/d/1yYyyGVlItfJ5G1m-3eYJYeg6p4Jce3TRxY83MQOrV74

Thanks @comphead. I will create a follow-on PR to add a text version of Parth's diagram. I have an AI generated version ready to submit.

@andygrove
Copy link
Member Author

Thanks @andygrove I would also include @parthchandra flow-class-landscape diagram how everything is interconnected https://docs.google.com/drawings/d/1yYyyGVlItfJ5G1m-3eYJYeg6p4Jce3TRxY83MQOrV74

Thanks @comphead. I will create a follow-on PR to add a text version of Parth's diagram. I have an AI generated version ready to submit.

#2681

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants