Don't panic on zero placeholder in ParamValues::get_placeholders_with_values (#8615)

It correctly rejected `$0` but not the other ones that are parsed
equally (e.g., `$000`).

Co-authored-by: Andrew Lamb <[email protected]>

Author: kawadakk

datafusion/common/src/param_value.rs

@@ -72,17 +72,20 @@ impl ParamValues {
     ) -> Result<ScalarValue> {
         match self {
             ParamValues::List(list) => {
-                if id.is_empty() || id == "$0" {
+                if id.is_empty() {
                     return _plan_err!("Empty placeholder id");
                 }
                 // convert id (in format $1, $2, ..) to idx (0, 1, ..)
-                let idx = id[1..].parse::<usize>().map_err(|e| {
-                    DataFusionError::Internal(format!(
-                        "Failed to parse placeholder id: {e}"
-                    ))
-                })? - 1;
+                let idx = id[1..]
+                    .parse::<usize>()
+                    .map_err(|e| {
+                        DataFusionError::Internal(format!(
+                            "Failed to parse placeholder id: {e}"
+                        ))
+                    })?
+                    .checked_sub(1);
                 // value at the idx-th position in param_values should be the value for the placeholder
-                let value = list.get(idx).ok_or_else(|| {
+                let value = idx.and_then(|idx| list.get(idx)).ok_or_else(|| {
                     DataFusionError::Internal(format!(
                         "No value found for placeholder with id {id}"
                     ))

datafusion/expr/src/logical_plan/plan.rs

@@ -3099,6 +3099,19 @@ digraph {
             .build()
             .unwrap();
 
+        plan.replace_params_with_values(&param_values.clone().into())
+            .expect_err("unexpectedly succeeded to replace an invalid placeholder");
+
+        // test $00 placeholder
+        let schema = Schema::new(vec![Field::new("id", DataType::Int32, false)]);
+
+        let plan = table_scan(TableReference::none(), &schema, None)
+            .unwrap()
+            .filter(col("id").eq(placeholder("$00")))
+            .unwrap()
+            .build()
+            .unwrap();
+
         plan.replace_params_with_values(&param_values.into())
             .expect_err("unexpectedly succeeded to replace an invalid placeholder");
     }