You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "KrishnaSudarshan7 (via GitHub)" <gi...@apache.org> on 2026/04/18 14:18:46 UTC
[PR] added support for MapFromEntries [datafusion]
KrishnaSudarshan7 opened a new pull request, #21720:
URL: https://github.com/apache/datafusion/pull/21720
## Which issue does this PR close?
Closes # (none — prerequisite for https://github.com/apache/datafusion-comet/issues/2706; follow-up to #17779 and #19274).
## Rationale for this change
The existing Spark map_from_entries UDF hardcoded LAST_WIN duplicate-key behavior and silently propagated null struct entries / null keys. Spark's default is EXCEPTION via spark.sql.mapKeyDedupPolicy, and Spark 4 raises SparkRuntimeException with error classes DUPLICATED_MAP_KEY / NULL_MAP_KEY. Without a configurable policy and matching error messages, downstream engines (e.g. datafusion-comet) must fall back to Spark for the common case.
## What changes are included in this PR?
1. Add execution.map_key_dedup_policy to ExecutionConfig in config.rs, mirroring the enable_ansi_mode pattern.
2. Introduce MapKeyDedupPolicy enum and thread it through the dedup helper in utils.rs; raise [DUPLICATED_MAP_KEY] ... under EXCEPTION.
3. Read the policy from ScalarFunctionArgs::config_options in map_from_entries.rs; validate null struct entries and null keys with [NULL_MAP_KEY] ...; preserve outer list-row nulls.
4. Pass EXCEPTION explicitly in map_from_arrays.rs.
## Are these changes tested?
Yes — four new Rust unit tests in map_from_entries.rs cover: happy path, duplicate under EXCEPTION, duplicate under LAST_WIN, null struct entry, and null key. Existing sqllogictest coverage for map_from_entries continues to pass.
## Are there any user-facing changes?
Yes — one new config key datafusion.execution.map_key_dedup_policy (default "EXCEPTION", accepts "LAST_WIN"). Error messages from map_from_entries now carry Spark-style bracketed error-class prefixes. No breaking API changes.
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "athlcode (via GitHub)" <gi...@apache.org>.
athlcode commented on code in PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#discussion_r3107303439
##########
datafusion/spark/src/function/map/utils.rs:
##########
@@ -202,17 +202,20 @@ fn map_deduplicate_keys(
cur_keys_offset + cur_entry_idx,
)?
.compacted();
+ // Enforce Spark's default `spark.sql.mapKeyDedupPolicy=EXCEPTION`.
+ // Native LAST_WIN support is deferred to a follow-up.
if seen_keys.contains(&key) {
- // TODO: implement configuration and logic for spark.sql.mapKeyDedupPolicy=EXCEPTION (this is default spark-config)
- // exec_err!("invalid argument: duplicate keys in map")
- // https://github.com/apache/spark/blob/cf3a34e19dfcf70e2d679217ff1ba21302212472/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala#L4961
- } else {
- // This code implements deduplication logic for spark.sql.mapKeyDedupPolicy=LAST_WIN (this is NOT default spark-config)
- keys_mask_one[cur_entry_idx] = true;
- values_mask_one[cur_entry_idx] = true;
- seen_keys.insert(key);
- new_last_offset += 1;
+ return exec_err!(
+ "[DUPLICATED_MAP_KEY] Duplicate map key {key} was found, \
+ please check the input data. If you want to remove the \
+ duplicated keys, you can set spark.sql.mapKeyDedupPolicy \
Review Comment:
changing the error geared towards DF
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#discussion_r3165868616
##########
datafusion/common/src/config.rs:
##########
@@ -692,6 +692,17 @@ config_namespace! {
/// `false` — ANSI SQL mode is disabled by default.
pub enable_ansi_mode: bool, default = false
+ /// Policy for handling duplicate keys in Spark-compatible map-construction
Review Comment:
I wonder if we should consider having a namespace for Spark specific configs, cc @andygrove @comphead
##########
datafusion/common/src/config.rs:
##########
@@ -692,6 +692,17 @@ config_namespace! {
/// `false` — ANSI SQL mode is disabled by default.
pub enable_ansi_mode: bool, default = false
+ /// Policy for handling duplicate keys in Spark-compatible map-construction
+ /// functions (`map_from_arrays`, `map_from_entries`, `str_to_map`).
+ ///
+ /// Mirrors Spark's
+ /// [`spark.sql.mapKeyDedupPolicy`](https://github.com/apache/spark/blob/cf3a34e19dfcf70e2d679217ff1ba21302212472/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala#L4961):
+ /// - `"EXCEPTION"` (default): raise `[DUPLICATED_MAP_KEY]` at runtime on any duplicate key.
+ /// - `"LAST_WIN"`: keep the last occurrence of each duplicate key.
+ ///
+ /// Values are case-insensitive. Only affects functions under `datafusion/spark`.
+ pub map_key_dedup_policy: String, default = "EXCEPTION".to_string()
Review Comment:
Would prefer having the enum value here along with the parsing, instead of only being a string
Reference
- https://github.com/apache/datafusion/issues/17498
##########
datafusion/spark/src/function/map/utils.rs:
##########
@@ -193,43 +216,193 @@ fn map_deduplicate_keys(
return exec_err!(
"map_deduplicate_keys: keys and values lists in the same row must have equal lengths"
);
- } else if num_keys_entries != 0 {
- let mut seen_keys = HashSet::new();
-
- for cur_entry_idx in (0..num_keys_entries).rev() {
- let key = ScalarValue::try_from_array(
- &flat_keys,
- cur_keys_offset + cur_entry_idx,
- )?
- .compacted();
- if seen_keys.contains(&key) {
- // TODO: implement configuration and logic for spark.sql.mapKeyDedupPolicy=EXCEPTION (this is default spark-config)
- // exec_err!("invalid argument: duplicate keys in map")
- // https://github.com/apache/spark/blob/cf3a34e19dfcf70e2d679217ff1ba21302212472/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala#L4961
- } else {
- // This code implements deduplication logic for spark.sql.mapKeyDedupPolicy=LAST_WIN (this is NOT default spark-config)
- keys_mask_one[cur_entry_idx] = true;
- values_mask_one[cur_entry_idx] = true;
- seen_keys.insert(key);
- new_last_offset += 1;
+ }
+ key_to_output_idx.clear();
+ for cur_entry_idx in 0..num_keys_entries {
+ let key = ScalarValue::try_from_array(
+ &flat_keys,
+ cur_keys_offset + cur_entry_idx,
+ )?
+ .compacted();
+ let abs_value_idx = (cur_values_offset + cur_entry_idx) as i32;
+
+ if let Some(&output_idx) = key_to_output_idx.get(&key) {
+ if last_value_wins {
+ value_indices[output_idx] = abs_value_idx;
+ keys_mask_builder.append_value(false);
+ continue;
}
+ return exec_err!(
+ "[DUPLICATED_MAP_KEY] Duplicate map key {key} was found, \
+ please check the input data. To allow duplicate keys with \
+ last-value-wins semantics, set \
+ `datafusion.execution.map_key_dedup_policy` to `LAST_WIN`."
+ );
}
+ keys_mask_builder.append_value(true);
+ key_to_output_idx.insert(key, value_indices.len());
+ value_indices.push(abs_value_idx);
+ new_last_offset += 1;
}
} else {
- // the result entry is NULL
- // both current row offsets are skipped
- // keys or values in the current row are marked false in the masks
+ // The result entry is NULL — no keys/values emitted. Still pad the
+ // mask so it stays aligned with `flat_keys`.
+ keys_mask_builder.append_n(num_keys_entries, false);
}
- keys_mask_builder.append_array(&keys_mask_one.into());
- values_mask_builder.append_array(&values_mask_one.into());
new_offsets.push(new_last_offset);
cur_keys_offset += num_keys_entries;
cur_values_offset += num_values_entries;
}
let keys_mask = keys_mask_builder.finish();
- let values_mask = values_mask_builder.finish();
let needed_keys = filter(&flat_keys, &keys_mask)?;
- let needed_values = filter(&flat_values, &values_mask)?;
+ let value_indices_array = Int32Array::from(value_indices);
+ let needed_values = take(&flat_values, &value_indices_array, None)?;
let offsets = OffsetBuffer::new(new_offsets.into());
Ok((needed_keys, needed_values, offsets))
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use arrow::array::{Int32Array, MapArray, StringArray};
+
+ fn as_map(array: &ArrayRef) -> &MapArray {
+ array.as_any().downcast_ref::<MapArray>().expect("MapArray")
+ }
+
+ fn int32_utf8_inputs(
+ keys: Vec<i32>,
+ values: Vec<Option<&str>>,
+ ) -> (ArrayRef, ArrayRef) {
+ let keys: ArrayRef = Arc::new(Int32Array::from(keys));
+ let values: ArrayRef = Arc::new(StringArray::from(values));
+ (keys, values)
+ }
+
+ #[test]
+ fn parse_policy_accepts_both_values_case_insensitively() {
+ assert!(!parse_map_key_dedup_policy("EXCEPTION").unwrap());
+ assert!(!parse_map_key_dedup_policy("exception").unwrap());
+ assert!(parse_map_key_dedup_policy("LAST_WIN").unwrap());
+ assert!(parse_map_key_dedup_policy("last_win").unwrap());
+ }
+
+ #[test]
+ fn parse_policy_rejects_unknown() {
+ let err = parse_map_key_dedup_policy("BOGUS").unwrap_err().to_string();
+ assert!(err.contains("Unknown map_key_dedup_policy"), "{err}");
+ }
+
+ #[test]
+ fn happy_path_two_rows_no_duplicates() {
+ let (keys, values) =
+ int32_utf8_inputs(vec![1, 2, 3], vec![Some("a"), Some("b"), Some("c")]);
+ let offsets = [0i32, 2, 3];
+
+ let result = map_from_keys_values_offsets_nulls(
+ &keys, &values, &offsets, &offsets, None, None, false,
+ )
+ .unwrap();
+
+ let map = as_map(&result);
+ assert_eq!(map.len(), 2);
+ assert_eq!(map.value_offsets(), &[0, 2, 3]);
+ }
+
+ #[test]
+ fn single_row_duplicate_errors_under_exception() {
+ let (keys, values) =
+ int32_utf8_inputs(vec![1, 2, 1], vec![Some("a"), Some("b"), Some("c")]);
+ let offsets = [0i32, 3];
+
+ let err = map_from_keys_values_offsets_nulls(
+ &keys, &values, &offsets, &offsets, None, None, false,
+ )
+ .unwrap_err()
+ .to_string();
+
+ assert!(err.contains("[DUPLICATED_MAP_KEY]"), "{err}");
+ assert!(err.contains("map_key_dedup_policy"), "{err}");
+ }
+
+ #[test]
+ fn last_win_keeps_final_occurrence() {
+ let (keys, values) = int32_utf8_inputs(
+ vec![1, 2, 1, 3, 2],
+ vec![Some("a"), Some("b"), Some("c"), Some("d"), Some("e")],
+ );
+ let offsets = [0i32, 5];
+
+ let result = map_from_keys_values_offsets_nulls(
+ &keys, &values, &offsets, &offsets, None, None, true,
+ )
+ .unwrap();
+
+ let map = as_map(&result);
+ assert_eq!(map.len(), 1);
+ // 5 entries in, 3 unique keys -> offsets [0, 3]
+ assert_eq!(map.value_offsets(), &[0, 3]);
+ }
+
+ #[test]
+ fn duplicate_in_later_row_still_errors() {
+ let (keys, values) = int32_utf8_inputs(
+ vec![1, 2, 1, 1],
+ vec![Some("a"), Some("b"), Some("x"), Some("y")],
+ );
+ let offsets = [0i32, 2, 4];
+
+ let err = map_from_keys_values_offsets_nulls(
+ &keys, &values, &offsets, &offsets, None, None, false,
+ )
+ .unwrap_err()
+ .to_string();
+
+ assert!(err.contains("[DUPLICATED_MAP_KEY]"), "{err}");
+ }
+
+ #[test]
+ fn empty_row_does_not_trigger_dedup() {
+ let (keys, values) = int32_utf8_inputs(vec![], vec![]);
+ let offsets = [0i32, 0];
+
+ let result = map_from_keys_values_offsets_nulls(
+ &keys, &values, &offsets, &offsets, None, None, false,
+ )
+ .unwrap();
+
+ let map = as_map(&result);
+ assert_eq!(map.len(), 1);
+ assert_eq!(map.value_offsets(), &[0, 0]);
+ }
+
+ #[test]
+ fn null_row_is_skipped_and_not_checked() {
+ // Row 0 is NULL (keys null). Its duplicate keys should be ignored;
+ // row 1 is a clean row.
+ let (keys, values) = int32_utf8_inputs(
+ vec![1, 1, 2, 3],
+ vec![Some("dup-a"), Some("dup-b"), Some("x"), Some("y")],
+ );
+ let offsets = [0i32, 2, 4];
+ let keys_nulls = NullBuffer::from(vec![false, true]);
+
+ let result = map_from_keys_values_offsets_nulls(
+ &keys,
+ &values,
+ &offsets,
+ &offsets,
+ Some(&keys_nulls),
+ None,
+ false,
+ )
+ .unwrap();
+
+ let map = as_map(&result);
Review Comment:
```suggestion
let map = result.as_map();
```
Using https://docs.rs/arrow/latest/arrow/array/trait.AsArray.html#method.as_map
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "coderfender (via GitHub)" <gi...@apache.org>.
coderfender commented on PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#issuecomment-4273954942
Thank you for your PR @KrishnaSudarshan7 . Could you add related tests on the spark side as well . Please refer to existing SLT tests in the repo for reference
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "athlcode (via GitHub)" <gi...@apache.org>.
athlcode commented on code in PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#discussion_r3311815062
##########
datafusion/common/src/config.rs:
##########
@@ -1461,6 +1508,24 @@ impl<'a> TryFrom<&'a FormatOptions> for arrow::util::display::FormatOptions<'a>
}
}
+config_namespace! {
Review Comment:
Looked into it, no existing precedent for feature-gating a top-level 'ConfigOptions' namespace. parquet_encryption is the only #[cfg] user in config.rs and it gates fields-within-a-struct, not the struct's registration.
##########
datafusion/common/src/config.rs:
##########
@@ -1461,6 +1508,24 @@ impl<'a> TryFrom<&'a FormatOptions> for arrow::util::display::FormatOptions<'a>
}
}
+config_namespace! {
Review Comment:
Looked into it, no existing precedent for feature-gating a top-level ConfigOptions namespace. parquet_encryption is the only #[cfg] user in config.rs and it gates fields-within-a-struct, not the struct's registration.
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "athlcode (via GitHub)" <gi...@apache.org>.
athlcode commented on PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#issuecomment-4346209012
> @athlcode , it seems like there are test failures here.
fixing the issues. Will tag once I fix and push, thank 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: [PR] added support for MapFromEntries [datafusion]
Posted by "athlcode (via GitHub)" <gi...@apache.org>.
athlcode commented on code in PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#discussion_r3203258298
##########
datafusion/spark/src/function/map/utils.rs:
##########
@@ -193,43 +216,193 @@ fn map_deduplicate_keys(
return exec_err!(
"map_deduplicate_keys: keys and values lists in the same row must have equal lengths"
);
- } else if num_keys_entries != 0 {
- let mut seen_keys = HashSet::new();
-
- for cur_entry_idx in (0..num_keys_entries).rev() {
- let key = ScalarValue::try_from_array(
- &flat_keys,
- cur_keys_offset + cur_entry_idx,
- )?
- .compacted();
- if seen_keys.contains(&key) {
- // TODO: implement configuration and logic for spark.sql.mapKeyDedupPolicy=EXCEPTION (this is default spark-config)
- // exec_err!("invalid argument: duplicate keys in map")
- // https://github.com/apache/spark/blob/cf3a34e19dfcf70e2d679217ff1ba21302212472/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala#L4961
- } else {
- // This code implements deduplication logic for spark.sql.mapKeyDedupPolicy=LAST_WIN (this is NOT default spark-config)
- keys_mask_one[cur_entry_idx] = true;
- values_mask_one[cur_entry_idx] = true;
- seen_keys.insert(key);
- new_last_offset += 1;
+ }
+ key_to_output_idx.clear();
+ for cur_entry_idx in 0..num_keys_entries {
+ let key = ScalarValue::try_from_array(
+ &flat_keys,
+ cur_keys_offset + cur_entry_idx,
+ )?
+ .compacted();
+ let abs_value_idx = (cur_values_offset + cur_entry_idx) as i32;
+
+ if let Some(&output_idx) = key_to_output_idx.get(&key) {
+ if last_value_wins {
+ value_indices[output_idx] = abs_value_idx;
+ keys_mask_builder.append_value(false);
+ continue;
}
+ return exec_err!(
+ "[DUPLICATED_MAP_KEY] Duplicate map key {key} was found, \
+ please check the input data. To allow duplicate keys with \
+ last-value-wins semantics, set \
+ `datafusion.execution.map_key_dedup_policy` to `LAST_WIN`."
+ );
}
+ keys_mask_builder.append_value(true);
+ key_to_output_idx.insert(key, value_indices.len());
+ value_indices.push(abs_value_idx);
+ new_last_offset += 1;
}
} else {
- // the result entry is NULL
- // both current row offsets are skipped
- // keys or values in the current row are marked false in the masks
+ // The result entry is NULL — no keys/values emitted. Still pad the
+ // mask so it stays aligned with `flat_keys`.
+ keys_mask_builder.append_n(num_keys_entries, false);
}
- keys_mask_builder.append_array(&keys_mask_one.into());
- values_mask_builder.append_array(&values_mask_one.into());
new_offsets.push(new_last_offset);
cur_keys_offset += num_keys_entries;
cur_values_offset += num_values_entries;
}
let keys_mask = keys_mask_builder.finish();
- let values_mask = values_mask_builder.finish();
let needed_keys = filter(&flat_keys, &keys_mask)?;
- let needed_values = filter(&flat_values, &values_mask)?;
+ let value_indices_array = Int32Array::from(value_indices);
+ let needed_values = take(&flat_values, &value_indices_array, None)?;
let offsets = OffsetBuffer::new(new_offsets.into());
Ok((needed_keys, needed_values, offsets))
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use arrow::array::{Int32Array, MapArray, StringArray};
+
+ fn as_map(array: &ArrayRef) -> &MapArray {
+ array.as_any().downcast_ref::<MapArray>().expect("MapArray")
+ }
+
+ fn int32_utf8_inputs(
+ keys: Vec<i32>,
+ values: Vec<Option<&str>>,
+ ) -> (ArrayRef, ArrayRef) {
+ let keys: ArrayRef = Arc::new(Int32Array::from(keys));
+ let values: ArrayRef = Arc::new(StringArray::from(values));
+ (keys, values)
+ }
+
+ #[test]
+ fn parse_policy_accepts_both_values_case_insensitively() {
+ assert!(!parse_map_key_dedup_policy("EXCEPTION").unwrap());
+ assert!(!parse_map_key_dedup_policy("exception").unwrap());
+ assert!(parse_map_key_dedup_policy("LAST_WIN").unwrap());
+ assert!(parse_map_key_dedup_policy("last_win").unwrap());
+ }
+
+ #[test]
+ fn parse_policy_rejects_unknown() {
+ let err = parse_map_key_dedup_policy("BOGUS").unwrap_err().to_string();
+ assert!(err.contains("Unknown map_key_dedup_policy"), "{err}");
+ }
+
+ #[test]
+ fn happy_path_two_rows_no_duplicates() {
+ let (keys, values) =
+ int32_utf8_inputs(vec![1, 2, 3], vec![Some("a"), Some("b"), Some("c")]);
+ let offsets = [0i32, 2, 3];
+
+ let result = map_from_keys_values_offsets_nulls(
+ &keys, &values, &offsets, &offsets, None, None, false,
+ )
+ .unwrap();
+
+ let map = as_map(&result);
+ assert_eq!(map.len(), 2);
+ assert_eq!(map.value_offsets(), &[0, 2, 3]);
+ }
+
+ #[test]
+ fn single_row_duplicate_errors_under_exception() {
+ let (keys, values) =
+ int32_utf8_inputs(vec![1, 2, 1], vec![Some("a"), Some("b"), Some("c")]);
+ let offsets = [0i32, 3];
+
+ let err = map_from_keys_values_offsets_nulls(
+ &keys, &values, &offsets, &offsets, None, None, false,
+ )
+ .unwrap_err()
+ .to_string();
+
+ assert!(err.contains("[DUPLICATED_MAP_KEY]"), "{err}");
+ assert!(err.contains("map_key_dedup_policy"), "{err}");
+ }
+
+ #[test]
+ fn last_win_keeps_final_occurrence() {
+ let (keys, values) = int32_utf8_inputs(
+ vec![1, 2, 1, 3, 2],
+ vec![Some("a"), Some("b"), Some("c"), Some("d"), Some("e")],
+ );
+ let offsets = [0i32, 5];
+
+ let result = map_from_keys_values_offsets_nulls(
+ &keys, &values, &offsets, &offsets, None, None, true,
+ )
+ .unwrap();
+
+ let map = as_map(&result);
+ assert_eq!(map.len(), 1);
+ // 5 entries in, 3 unique keys -> offsets [0, 3]
+ assert_eq!(map.value_offsets(), &[0, 3]);
+ }
+
+ #[test]
+ fn duplicate_in_later_row_still_errors() {
+ let (keys, values) = int32_utf8_inputs(
+ vec![1, 2, 1, 1],
+ vec![Some("a"), Some("b"), Some("x"), Some("y")],
+ );
+ let offsets = [0i32, 2, 4];
+
+ let err = map_from_keys_values_offsets_nulls(
+ &keys, &values, &offsets, &offsets, None, None, false,
+ )
+ .unwrap_err()
+ .to_string();
+
+ assert!(err.contains("[DUPLICATED_MAP_KEY]"), "{err}");
+ }
+
+ #[test]
+ fn empty_row_does_not_trigger_dedup() {
+ let (keys, values) = int32_utf8_inputs(vec![], vec![]);
+ let offsets = [0i32, 0];
+
+ let result = map_from_keys_values_offsets_nulls(
+ &keys, &values, &offsets, &offsets, None, None, false,
+ )
+ .unwrap();
+
+ let map = as_map(&result);
+ assert_eq!(map.len(), 1);
+ assert_eq!(map.value_offsets(), &[0, 0]);
+ }
+
+ #[test]
+ fn null_row_is_skipped_and_not_checked() {
+ // Row 0 is NULL (keys null). Its duplicate keys should be ignored;
+ // row 1 is a clean row.
+ let (keys, values) = int32_utf8_inputs(
+ vec![1, 1, 2, 3],
+ vec![Some("dup-a"), Some("dup-b"), Some("x"), Some("y")],
+ );
+ let offsets = [0i32, 2, 4];
+ let keys_nulls = NullBuffer::from(vec![false, true]);
+
+ let result = map_from_keys_values_offsets_nulls(
+ &keys,
+ &values,
+ &offsets,
+ &offsets,
+ Some(&keys_nulls),
+ None,
+ false,
+ )
+ .unwrap();
+
+ let map = as_map(&result);
Review Comment:
thank you for the suggestion, pushed the changes
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "athlcode (via GitHub)" <gi...@apache.org>.
athlcode commented on PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#issuecomment-4290845068
@coderfender I agree with @Jefffrey as well, thank you. The way to do this would be to use datafusion.execution.map_key_dedup_policy config as @nuno-faria mentioned earlier.
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "athlcode (via GitHub)" <gi...@apache.org>.
athlcode commented on PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#issuecomment-4500905203
hey @Jefffrey. @nuno-faria has requested a review from you, thank 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: [PR] added support for MapFromEntries [datafusion]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#issuecomment-4399047696
<!-- semver-check-comment -->
Thank you for opening this pull request!
Reviewer note: [cargo-semver-checks](https://github.com/obi1kenobi/cargo-semver-checks) reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).
<details>
<summary>Details</summary>
```
Cloning apache/main
Building datafusion-common v53.1.0 (current)
Built [ 34.615s] (current)
Parsing datafusion-common v53.1.0 (current)
Parsed [ 0.061s] (current)
Building datafusion-common v53.1.0 (baseline)
Built [ 33.841s] (baseline)
Parsing datafusion-common v53.1.0 (baseline)
Parsed [ 0.061s] (baseline)
Checking datafusion-common v53.1.0 -> v53.1.0 (no change; assume patch)
Checked [ 0.868s] 222 checks: 221 pass, 1 fail, 0 warn, 30 skip
--- failure constructible_struct_adds_field: externally-constructible struct adds field ---
Description:
A pub struct constructible with a struct literal has a new pub field. Existing struct literals must be updated to include the new field.
ref: https://doc.rust-lang.org/reference/expressions/struct-expr.html
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.47.0/src/lints/constructible_struct_adds_field.ron
Failed in:
field ExecutionOptions.map_key_dedup_policy in /home/runner/work/datafusion/datafusion/datafusion/common/src/config.rs:469
Summary semver requires new major version: 1 major and 0 minor checks failed
Finished [ 70.747s] datafusion-common
Building datafusion-spark v53.1.0 (current)
Built [ 55.748s] (current)
Parsing datafusion-spark v53.1.0 (current)
Parsed [ 0.072s] (current)
Building datafusion-spark v53.1.0 (baseline)
Built [ 57.078s] (baseline)
Parsing datafusion-spark v53.1.0 (baseline)
Parsed [ 0.063s] (baseline)
Checking datafusion-spark v53.1.0 -> v53.1.0 (no change; assume patch)
Checked [ 0.429s] 222 checks: 222 pass, 30 skip
Summary no semver update required
Finished [ 114.953s] datafusion-spark
Building datafusion-sqllogictest v53.1.0 (current)
Built [ 140.695s] (current)
Parsing datafusion-sqllogictest v53.1.0 (current)
Parsed [ 0.024s] (current)
Building datafusion-sqllogictest v53.1.0 (baseline)
Built [ 141.561s] (baseline)
Parsing datafusion-sqllogictest v53.1.0 (baseline)
Parsed [ 0.025s] (baseline)
Checking datafusion-sqllogictest v53.1.0 -> v53.1.0 (no change; assume patch)
Checked [ 0.103s] 222 checks: 222 pass, 30 skip
Summary no semver update required
Finished [ 285.358s] datafusion-sqllogictest
```
</details>
<!-- semver-check-comment -->
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey merged PR #21720:
URL: https://github.com/apache/datafusion/pull/21720
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "coderfender (via GitHub)" <gi...@apache.org>.
coderfender commented on code in PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#discussion_r3107149796
##########
datafusion/spark/src/function/map/utils.rs:
##########
@@ -202,17 +202,20 @@ fn map_deduplicate_keys(
cur_keys_offset + cur_entry_idx,
)?
.compacted();
+ // Enforce Spark's default `spark.sql.mapKeyDedupPolicy=EXCEPTION`.
+ // Native LAST_WIN support is deferred to a follow-up.
if seen_keys.contains(&key) {
- // TODO: implement configuration and logic for spark.sql.mapKeyDedupPolicy=EXCEPTION (this is default spark-config)
- // exec_err!("invalid argument: duplicate keys in map")
- // https://github.com/apache/spark/blob/cf3a34e19dfcf70e2d679217ff1ba21302212472/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala#L4961
- } else {
- // This code implements deduplication logic for spark.sql.mapKeyDedupPolicy=LAST_WIN (this is NOT default spark-config)
- keys_mask_one[cur_entry_idx] = true;
- values_mask_one[cur_entry_idx] = true;
- seen_keys.insert(key);
- new_last_offset += 1;
+ return exec_err!(
+ "[DUPLICATED_MAP_KEY] Duplicate map key {key} was found, \
+ please check the input data. If you want to remove the \
+ duplicated keys, you can set spark.sql.mapKeyDedupPolicy \
Review Comment:
We might want to keep the error message but might make the error more geared towards DF
##########
datafusion/sqllogictest/test_files/spark/map/map_from_entries.slt:
##########
@@ -151,14 +151,12 @@ SELECT
----
{outer_key1: {inner_a: 1, inner_b: 2}, outer_key2: {inner_x: 10, inner_y: 20, inner_z: 30}}
-# Test with duplicate keys
-query ?
+# Test with duplicate keys: raises DUPLICATED_MAP_KEY under Spark's default policy
+query error DataFusion error: Execution error: \[DUPLICATED_MAP_KEY\] Duplicate map key true was found
SELECT map_from_entries(array(
- struct(true, 'a'),
- struct(false, 'b'),
+ struct(true, 'a'),
+ struct(false, 'b'),
struct(true, 'c'),
- struct(false, cast(NULL as string)),
+ struct(false, cast(NULL as string)),
struct(true, 'd')
Review Comment:
Might want to revert unwanted formatting changes here
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "coderfender (via GitHub)" <gi...@apache.org>.
coderfender commented on PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#issuecomment-4291063051
Sure @athlcode please take a look at the current config naming conventions to keep them consistent
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "coderfender (via GitHub)" <gi...@apache.org>.
coderfender commented on code in PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#discussion_r3186527944
##########
datafusion/common/src/config.rs:
##########
@@ -692,6 +692,17 @@ config_namespace! {
/// `false` — ANSI SQL mode is disabled by default.
pub enable_ansi_mode: bool, default = false
+ /// Policy for handling duplicate keys in Spark-compatible map-construction
Review Comment:
This makes sense to me. A few weeks ago , I was proposing semantic layering in DF essentially calling spark functions with same name as datafusion , spark level config , ansi support and potentially spark friendly SQL. This should help users migrating from spark to directly move their leverage logic in datafusion
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "coderfender (via GitHub)" <gi...@apache.org>.
coderfender commented on PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#issuecomment-4276345499
https://spark.apache.org/docs/latest/sql-ref-ansi-compliance.html
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "nuno-faria (via GitHub)" <gi...@apache.org>.
nuno-faria commented on code in PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#discussion_r3106602830
##########
datafusion/spark/src/function/map/utils.rs:
##########
@@ -202,17 +202,20 @@ fn map_deduplicate_keys(
cur_keys_offset + cur_entry_idx,
)?
.compacted();
+ // Enforce Spark's default `spark.sql.mapKeyDedupPolicy=EXCEPTION`.
+ // Native LAST_WIN support is deferred to a follow-up.
if seen_keys.contains(&key) {
- // TODO: implement configuration and logic for spark.sql.mapKeyDedupPolicy=EXCEPTION (this is default spark-config)
- // exec_err!("invalid argument: duplicate keys in map")
- // https://github.com/apache/spark/blob/cf3a34e19dfcf70e2d679217ff1ba21302212472/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala#L4961
- } else {
- // This code implements deduplication logic for spark.sql.mapKeyDedupPolicy=LAST_WIN (this is NOT default spark-config)
- keys_mask_one[cur_entry_idx] = true;
- values_mask_one[cur_entry_idx] = true;
- seen_keys.insert(key);
- new_last_offset += 1;
+ return exec_err!(
+ "[DUPLICATED_MAP_KEY] Duplicate map key {key} was found, \
+ please check the input data. If you want to remove the \
+ duplicated keys, you can set spark.sql.mapKeyDedupPolicy \
Review Comment:
This error message points to the `spark.sql.mapKeyDedupPolicy` config, which is Spark specific and does not exist in DataFusion.
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#issuecomment-4594107195
Thanks @athlcode, @nuno-faria & @coderfender
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "coderfender (via GitHub)" <gi...@apache.org>.
coderfender commented on PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#issuecomment-4337874330
@athlcode , it seems like there are test failures here.
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "athlcode (via GitHub)" <gi...@apache.org>.
athlcode commented on code in PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#discussion_r3335051876
##########
docs/source/library-user-guide/upgrading/54.0.0.md:
##########
@@ -596,6 +596,37 @@ impl Default for MyTreeNode {
}
```
+### Spark map functions now reject duplicate keys by default
Review Comment:
done, thank 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: [PR] added support for MapFromEntries [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#issuecomment-4286108203
Yes I think it would be a good idea to make this configurable instead of just switching the behaviour (otherwise we're in a loop of having TODO item to support the other behaviour)
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#discussion_r3287258303
##########
datafusion/common/src/config.rs:
##########
@@ -1461,6 +1508,24 @@ impl<'a> TryFrom<&'a FormatOptions> for arrow::util::display::FormatOptions<'a>
}
}
+config_namespace! {
Review Comment:
I do wonder if we should feature gate this, but I don't know if we have an existing precedent for doing do 🤔
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "coderfender (via GitHub)" <gi...@apache.org>.
coderfender commented on PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#issuecomment-4290431492
I agree with @Jefffrey unless you would have an immediate follow up @athlcode ?
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "athlcode (via GitHub)" <gi...@apache.org>.
athlcode commented on code in PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#discussion_r3311815062
##########
datafusion/common/src/config.rs:
##########
@@ -1461,6 +1508,24 @@ impl<'a> TryFrom<&'a FormatOptions> for arrow::util::display::FormatOptions<'a>
}
}
+config_namespace! {
Review Comment:
Looked into it, no existing precedent for feature-gating a top-level ConfigOptions namespace. parquet_encryption is the only #[cfg] user in config.rs and it gates fields-within-a-struct, not the struct's registration.
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "athlcode (via GitHub)" <gi...@apache.org>.
athlcode commented on PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#issuecomment-4413488893
@Jefffrey @nuno-faria
Please do let me know if the changes are good or have any suggestions, thank 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: [PR] added support for MapFromEntries [datafusion]
Posted by "athlcode (via GitHub)" <gi...@apache.org>.
athlcode commented on PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#issuecomment-4276562853
> Thanks @athlcode. Since this changes the current behavior, I think it would be a good opportunity to add a config that allows switching between exception and last key wins (e.g., `datafusion.execution.mapKeyDedupPolicy`). Otherwise existing code relying on the last key wins policy won't be able to adapt.
>
> cc: @Jefffrey and @comphead since you both reviewed the original `map_from_entries`.
thanks @nuno-faria. I am looking into this as well
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "athlcode (via GitHub)" <gi...@apache.org>.
athlcode commented on code in PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#discussion_r3203752534
##########
datafusion/common/src/config.rs:
##########
@@ -692,6 +692,17 @@ config_namespace! {
/// `false` — ANSI SQL mode is disabled by default.
pub enable_ansi_mode: bool, default = false
+ /// Policy for handling duplicate keys in Spark-compatible map-construction
Review Comment:
@coderfender @Jefffrey thank you, updated to namespace
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "athlcode (via GitHub)" <gi...@apache.org>.
athlcode commented on code in PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#discussion_r3107409857
##########
datafusion/sqllogictest/test_files/spark/map/map_from_entries.slt:
##########
@@ -151,14 +151,12 @@ SELECT
----
{outer_key1: {inner_a: 1, inner_b: 2}, outer_key2: {inner_x: 10, inner_y: 20, inner_z: 30}}
-# Test with duplicate keys
-query ?
+# Test with duplicate keys: raises DUPLICATED_MAP_KEY under Spark's default policy
+query error DataFusion error: Execution error: \[DUPLICATED_MAP_KEY\] Duplicate map key true was found
SELECT map_from_entries(array(
- struct(true, 'a'),
- struct(false, 'b'),
+ struct(true, 'a'),
+ struct(false, 'b'),
struct(true, 'c'),
- struct(false, cast(NULL as string)),
+ struct(false, cast(NULL as string)),
struct(true, 'd')
Review Comment:
done
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "athlcode (via GitHub)" <gi...@apache.org>.
athlcode commented on PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#issuecomment-4276544425
> The PR looks good @athlcode . Left some comments and I would like to see some more tests covering various edge cases both on. the rust side and SLT side to add more confidence
yep fixing comments and will add tests
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "athlcode (via GitHub)" <gi...@apache.org>.
athlcode commented on code in PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#discussion_r3203268047
##########
datafusion/common/src/config.rs:
##########
@@ -692,6 +692,17 @@ config_namespace! {
/// `false` — ANSI SQL mode is disabled by default.
pub enable_ansi_mode: bool, default = false
+ /// Policy for handling duplicate keys in Spark-compatible map-construction
+ /// functions (`map_from_arrays`, `map_from_entries`, `str_to_map`).
+ ///
+ /// Mirrors Spark's
+ /// [`spark.sql.mapKeyDedupPolicy`](https://github.com/apache/spark/blob/cf3a34e19dfcf70e2d679217ff1ba21302212472/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala#L4961):
+ /// - `"EXCEPTION"` (default): raise `[DUPLICATED_MAP_KEY]` at runtime on any duplicate key.
+ /// - `"LAST_WIN"`: keep the last occurrence of each duplicate key.
+ ///
+ /// Values are case-insensitive. Only affects functions under `datafusion/spark`.
+ pub map_key_dedup_policy: String, default = "EXCEPTION".to_string()
Review Comment:
thank you, added MapKeyDedupPolicy enum with Exception (default) + LastWin
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "athlcode (via GitHub)" <gi...@apache.org>.
athlcode commented on PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#issuecomment-4415333150
> Thanks @athlcode. I think the only think missing now is adding an entry to the upgrade guide (docs/source/library-user-guide/upgrading/54.0.0.md), since the default behavior has changed, showing how to revert back to it.
thanks @nuno-faria. I have added an entry to the upgrade guide
--
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: [PR] added support for MapFromEntries [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #21720:
URL: https://github.com/apache/datafusion/pull/21720#discussion_r3334954696
##########
docs/source/library-user-guide/upgrading/54.0.0.md:
##########
@@ -596,6 +596,37 @@ impl Default for MyTreeNode {
}
```
+### Spark map functions now reject duplicate keys by default
Review Comment:
i think we need to move this to v55 guide now that v54 branch has been cut
--
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