Github user cloud-fan commented on a diff in the pull request:
    --- Diff: 
    @@ -35,15 +35,14 @@ case class DataSourceV2Relation(
         options: Map[String, String],
         projection: Seq[AttributeReference],
         filters: Option[Seq[Expression]] = None,
    -    userSpecifiedSchema: Option[StructType] = None) extends LeafNode with 
MultiInstanceRelation {
    +    userSpecifiedSchema: Option[StructType] = None)
    +  extends LeafNode with MultiInstanceRelation with DataSourceV2QueryPlan {
    --- End diff --
    Sorry I didn't realize you removed the extending of 
`DataSourceReaderHolder`, otherwise I would point it out in your PR. In general 
the question is, how should we define the equality of data source relation? It 
was defined by `output, reader.getClass, filters` before, and your PR changed 
it to the default equality of `DataSourceV2Relation` silently.
    The major difference is, should `options` take part in the equality? The 
answer is obviously yes, like `path`, so I'll add `options` to the equality.
    BTW `DataSourceV2QueryPlan` is needed, as there are 3 plans need to 
implement explain: `DataSourceV2Relation`, `StreamingDataSourceV2Relation`, 
    > This doesn't use the source name if it is named
    I missed that, let me add back.
    > doesn't indicate that the source is v2
    I'll improve it
    > it doesn't show the most important part of the scan
    Do you mean the path option? First I don't think showing all the options is 
a good idea, as it can be a lot. My future plan is to show these standard 
options like path, table, etc. Again this is something added silently, there is 
no consensus about how to explain a data source v2 relation, my PR tries to 
have people focus on this part and have a consensus.


To unsubscribe, e-mail:
For additional commands, e-mail:

Reply via email to