Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/20647#discussion_r170162261
  
    --- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Relation.scala
 ---
    @@ -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`, 
`DataSourceV2ScanExec`.
    
    > 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: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to