Merge branch 'develop' into operators-bench

Author: joseph-isaacs

.github/workflows/bench-pr.yml

@@ -22,7 +22,6 @@ permissions:
   contents: read
   pull-requests: write  # for commenting on PRs
   id-token: write  # enables AWS-GitHub OIDC
-  deployments: write  # for Polar Signals profiling
 
 jobs:
   label_trigger:

.github/workflows/bench.yml

@@ -10,7 +10,6 @@ permissions:
   id-token: write  # enables AWS-GitHub OIDC
   actions: read
   contents: write
-  deployments: write
 
 jobs:
   commit-metadata:

.github/workflows/docs.yml

@@ -48,6 +48,8 @@ jobs:
           path: docs/_build/html
 
   deploy:
+    permissions:
+      deployments: write
     environment:
       name: github-pages
       url: ${{ steps.deployment.outputs.page_url }}

.github/workflows/nightly-bench.yml

@@ -15,7 +15,6 @@ permissions:
   id-token: write  # enables AWS-GitHub OIDC
   actions: read
   contents: write
-  deployments: write
 
 jobs:
   sql:

.github/workflows/publish.yml

@@ -46,6 +46,7 @@ jobs:
     timeout-minutes: 120
     permissions:
       id-token: write  # IMPORTANT: mandatory for trusted publishing
+      deployments: write
     environment:
       name: push-to-pypi
       url: https://pypi.org/p/vortex-data

.github/workflows/sql-pr.yml

@@ -17,7 +17,6 @@ permissions:
   contents: read
   pull-requests: write  # for commenting on PRs
   id-token: write  # enables AWS-GitHub OIDC
-  deployments: write  # for Polar Signals profiling
 
 jobs:
   label_trigger:

vortex-layout/src/layouts/dict/reader.rs

@@ -92,6 +92,7 @@ impl DictReader {
         // We capture the name, so it may be wrong if we re-use the same reader within multiple
         // different parent readers. But that's rare...
         let values_len = self.values_len;
+        let session = self.session.clone();
         self.values_array
             .get_or_init(move || {
                 self.values
@@ -102,7 +103,15 @@ impl DictReader {
                     )
                     .vortex_expect("must construct dict values array evaluation")
                     .map_err(Arc::new)
-                    .map_ok(|arr| arr.to_canonical().into_array())
+                    .map(move |array| {
+                        if *USE_VORTEX_OPERATORS {
+                            // We execute the array to avoid re-evaluating for every split.
+                            let array = array?;
+                            Ok(array.execute_vector(&session)?.into_array(array.dtype()))
+                        } else {
+                            Ok(array?.to_canonical().into_array())
+                        }
+                    })
                     .boxed()
                     .shared()
             })

vortex-layout/src/layouts/flat/reader.rs

@@ -37,16 +37,6 @@ use crate::segments::SegmentSource;
 //  actual expression? Perhaps all expressions are given a selection mask to decide for themselves?
 const EXPR_EVAL_THRESHOLD: f64 = 0.2;
 
-/// Below this mask density we will propagate filters one by one. In other words, we filter an
-/// array using a mask prior to running a filter expression, and then have to perform a more
-/// expensive rank intersection on the result. This threshold exists because filtering has a
-/// non-trivial cost, and often that cost outweighs evaluating the filter expression over a few
-/// more rows that are already known to be false.
-///
-/// TODO(ngates): this threshold should really be estimated based on the cost of the filter + the
-///  the cost of the expression itself.
-const FILTER_OF_FILTER_THRESHOLD: f64 = 0.8;
-
 pub struct FlatReader {
     layout: FlatLayout,
     name: Arc<str>,
@@ -157,7 +147,7 @@ impl LayoutReader for FlatReader {
             }
 
             let array_mask = if *USE_VORTEX_OPERATORS {
-                if mask.density() < FILTER_OF_FILTER_THRESHOLD {
+                if mask.density() < EXPR_EVAL_THRESHOLD {
                     // We have the choice to apply the filter or the expression first, we apply the
                     // expression first so that it can try pushing down itself and then the filter
                     // after this.