You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2024/04/30 16:57:59 UTC

[I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

alamb opened a new issue, #10316:
URL: https://github.com/apache/datafusion/issues/10316

   ### Is your feature request related to a problem or challenge?
   
   When merging a large number of pre-sorted streams (e.g. in our case, a large number of pre-sorted parquet files) the actual work in `SortPreservingMerge` to keep them sorted is often substantial  (as the sort key of each row in each stream must be compared the other potential candidates)
   
   Here is the sort preserving merge
   https://github.com/apache/datafusion/blob/4edbdd7d09d97f361748c086afbd7b3dda972f76/datafusion/physical-plan/src/sorts/sort_preserving_merge.rs#L39-L67
   
   However, in some cases (such as @suremarc  has identified in https://github.com/apache/datafusion/issues/6672) we can use information about how the values of the sort key columns are distributed to avoid needing a sort
   
   For example, if we have three files that are each sorted by `time` and have the following ranges
   * file1.paquet `min(time) = 2024-01-01` and `max(time) = 2024-01-31`
   * file2.paquet `min(time) = 2024-02-01` and `max(time) = 2024-02-28`
   * file3.paquet `min(time) = 2024-03-01` and `max(time) = 2024-03-31`
   
   We can produce the output sorted stream by first reading file1.parquet entirely then file2.parquet, then file3.parquet
   
   Not only will this be faster than using `SortPreservingMerge` it will require less intermediate memory as we don't need to read a batch from each input stream to begin producing output. For cases where there may be 100s of files, this can minimize the amount of concurrently outstanding requests substantially
   
   Also, for a query that will not read the entire dataset (e.g. only wants the most recent values) it can be especially beneficial:
   
   ```sql
   SELECT * FROM data ORDER BY time limit 10
   ```
   
   In this case our example above would only ever read file1.parquet (wouldn't even open the others) if it had more than 10 rows
   
   ### Describe the solution you'd like
   
   _No response_
   
   ### Describe alternatives you've considered
   
   _No response_
   
   ### Additional context
   
   The original inspiration for this operator came from @pauldix  (who I think mentioned it was inspired by ElasticSearch)


-- 
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: github-unsubscribe@datafusion.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2086188449

   FYI @ozankabak @mustafasrepo @matthewmturner 


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2108292710

   > @alamb : I am starting porting `ProgresssiveEval` from IOx. Do you want me to use this ticket or #10433 for that work or you want me to open a new ticket for the porting?
   
   How about we open a new ticketfor porting and link it to this epic?


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "NGA-TRAN (via GitHub)" <gi...@apache.org>.
NGA-TRAN commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2108082485

   @alamb : I am starting porting `ProgresssiveEval` from IOx. Do you want me to use this ticket or https://github.com/apache/datafusion/issues/10433 for that work or you want me to open a new ticket for the porting?


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "berkaysynnada (via GitHub)" <gi...@apache.org>.
berkaysynnada commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2719004069

   Hello @suremarc. @cj-zhukov is currently working on that issue, and we plan to propose it possibly next week. Once that happens, we will definitely ask for a review. If you wonder the process, perhaps @cj-zhukov can share an update with you.


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2086008585

   Here is the entire implementation in case anyone wants this: 
   [progressive_eval.zip](https://github.com/apache/datafusion/files/15168337/progressive_eval.zip)
   


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-5735134664

   ProgressiveEval is sounding like it may be time for a comeback


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2679231481

   FWIW I am working on the general analysis to support this operator for some unrelated reason in InfluxDB -- I plan to propose upstreaming it when complete. 


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2719209290

   I filed a ticket with the design we are thinking about:
   - https://github.com/apache/datafusion/issues/15191
   
   I also started collecting related tickets to the top of this one


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "NGA-TRAN (via GitHub)" <gi...@apache.org>.
NGA-TRAN commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2108411751

   This is the ticket to port `ProgressiveEval` from InfluxDB https://github.com/apache/datafusion/issues/10488


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2113301756

   Update here:
   
   1. @NGA-TRAN  has a PR open to port this operator into DataFusion: https://github.com/apache/datafusion/pull/10490
   2. We don't want to merge it into DataFusion until at least one other user could get value from it. 
   
   So in my mind, what is required to move on with this ticket is:
   1. Update one of the existing optimizer passes to detect when the sort keys of a `SortPreservingMerge` don't overlap in value and replace with `ProgressiveEval`
   2. Tests showing it working
   
   For the "do key ranges overlap" detection code I think we can use what @suremarc  added in https://github.com/apache/datafusion/pull/9593
   
   


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2086175393

   I probably should have mentioned that in order to use this operator, one needs to be able to determine if the input streams to `SortPreservingMerge` are non overlapping in value. Conveniently, that is basically the analysis that @suremarc  has implemented in https://github.com/apache/datafusion/pull/9593


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "suremarc (via GitHub)" <gi...@apache.org>.
suremarc commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2462945990

   I went ahead and made an attempt at implementing this over in this draft PR: #13296
   
   I was able to reuse the `MinMaxStatistics` code with some small changes which was pretty cool. 
   
   I have no nontrivial way to test this code until we get statistics per partition. In the PR I proposed the following API:
   
   ```rust
   pub trait ExecutionPlan: [...] {
       // [...]
       
       fn statistics_by_partition(&self) -> Result<Vec<Statistics>> {
           // Return global statistics by default
           Ok(vec![
               self.statistics()?;
               self.properties().partitioning.partition_count()
           ])
       }
   }
   ```
   
   In order for the statistics to be useful we'll actually need non-default implementations of course. So I'm wondering if I should just implement this method just for `ParquetExec` in my PR, so I can get some tests in. Of course, nothing will be finalized yet. 
   
   As discussed in [Epic: Statistics Improvements](https://github.com/apache/datafusion/issues/8227#issuecomment-2457565135) we will need #8078 in order for this code to actually work properly in all situations, but I believe @alamb is working on that. 


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2465530060

   > In order for the statistics to be useful we'll actually need non-default implementations of course. So I'm wondering if I should just implement this method just for ParquetExec in my PR, so I can get some tests in. Of course, nothing will be finalized yet.
   
   It might be a good idea to plumb it in to see how easy/hard it is
   
   I am trying to think of a way to avoid the per-partition statistics but I am currently drawing a blank


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "suremarc (via GitHub)" <gi...@apache.org>.
suremarc commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2718940819

   > FWIW I am working on the general analysis to support this operator for some unrelated reason in InfluxDB -- I plan to propose upstreaming it when complete.
   
   Hey @alamb, I've been catching up on the `StatisticsV2` work recently. I was planning to work on any remaining tasks for this ticket with @xudong963's help, however it looks like someone is working on #14896 and you are working on this one. This is an important feature for my team so we're happy to help out wherever we can. 


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "suremarc (via GitHub)" <gi...@apache.org>.
suremarc commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2447792748

   Hey @alamb, I am interested in using `ProgressiveEval` (or having the functionality merged into `SortPreservingMergeStream`) as my use case is optimizing unions where the children have non-overlapping ranges. For instance:
   ```sql
   SELECT * FROM t1 
   WHERE "timestamp" < cutoff
   UNION ALL
   SELECT * FROM t2
   WHERE "timestamp" >= cutoff
   ORDER BY "timestamp"
   ```
   
   In principle it should be possible to read results from `t1` first. 
   
   I do think we can reuse the analysis in #9593, but ideally we would have statistics per partition as @ozankabak mentioned. This would allow us to implement the operator generically, without really having to inspect its children. 


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2091665546

   > What comes to my mind is that if we can successfully bring the `statistics` to `SortPreservingMerge`, it could handle this work without a separate operator (This will probably need a change in `statistics` API to return statistics for each partition).
   
   I think this approach would work as well -- or maybe just a flag on the `SortPreservingMerge` and a different stream rather than an entirely `ExecutionPlan` 🤔 
   
   Per partition statistics is an interesting idea 🤔  -- it certainly makes sense for things like data sources. I wonder how generally useful it could be 


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2719181551

   > Hey @alamb, I've been catching up on the StatisticsV2 work recently. I was planning to work on any remaining tasks for this ticket with @xudong963's help, however it looks like someone is working on https://github.com/apache/datafusion/issues/14896 and you are working on this one. This is an important feature for my team so we're happy to help out wherever we can.
   
   @wiedld  has taken over working on this for us at InfluxData and I think she is making good progress. I will find the design I wrote up for that internal project and make a new public ticket to give you a sense of what we are working on in case it is helpful


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "pauldix (via GitHub)" <gi...@apache.org>.
pauldix commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2086162502

   Oh yeah, we're going to be hitting this all the time. Definitely want to see this! 😁 


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2086368541

   I think we should both port ProgressiveEval as well as hook it up in the optimizer so it is is used (likely based on the analysis in https://github.com/apache/datafusion/pull/9593)


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "NGA-TRAN (via GitHub)" <gi...@apache.org>.
NGA-TRAN commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2086302545

   Thanks @alamb. I am happy to port `ProgressiveEval` from InfluxDB to DB


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2086186833

   > Oh yeah, we're going to be hitting this all the time. Definitely want to see this! 😁
   
   Also to be clear to everyone else -- we have this code in InfluxDB already but it would be great to have other people be able to use it (and help maintain it)


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "suremarc (via GitHub)" <gi...@apache.org>.
suremarc commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2087191235

   @alamb Just to be absolutely clear, if the plan consists entirely of Parquet files from a single table, then the `SortPreservingMerge` will be eliminated. As you can see in [this sqllogictest](https://github.com/apache/datafusion/blob/aa8943323618e5761cd0cd361839c007a4572a81/datafusion/sqllogictest/test_files/parquet_sorted_statistics.slt#L96-L108), a single file group is produced if none of the files overlap, and DataFusion correctly understands a `SortPreservingMergeExec` is not required. In particular, after #9593, #6672 should be fixed. 
   
   That said, it's worth pointing out that `ProgressiveEval` is more general (it can work for arbitrary plans e.g. heterogeneous unions). 


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2770303558

   After thinking about it more, all this work is in service of optimizing certain types of queries, which @simonvandel  explained well on this ticket
   - https://github.com/apache/datafusion/issues/6672
   
   Thus I think we should use https://github.com/apache/datafusion/issues/6672 as the coordination point


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "NGA-TRAN (via GitHub)" <gi...@apache.org>.
NGA-TRAN commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2086435973

   If the hooking work is not that tricky (which I think the case), I am happy to do that, too


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "berkaysynnada (via GitHub)" <gi...@apache.org>.
berkaysynnada commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-2089710554

   What comes to my mind is that if we can successfully bring the `statistics` to `SortPreservingMerge`, it could handle this work without a separate operator (This will probably need a change in `statistics` API to return statistics for each partition). 


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Optimized version of `SortPreservingMerge` that doesn't actually compare sort keys of the key ranges are ordered [datafusion]

Posted by "stuhood (via GitHub)" <gi...@apache.org>.
stuhood commented on issue #10316:
URL: https://github.com/apache/datafusion/issues/10316#issuecomment-5721439575

   With `Partitioning::Range` landed, the physical plan can guarantee that partition streams are non-overlapping and ordered. In addition to (?) inspecting runtime file statistics, `SortPreservingMergeExec` could check for matching `Partitioning::Range` to stream partitions sequentially and early-exit on `LIMIT`.
   
   See #25437 (part of #25421).


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org