Nayana-Naik73 commented on code in PR #53521:
URL: https://github.com/apache/arrow/pull/53521#discussion_r4468469956
##########
cpp/src/arrow/engine/substrait/serde_test.cc:
##########
@@ -3147,6 +3147,77 @@ TEST(SubstraitRoundTrip, ReadRelWithEmit) {
/*include_columns=*/{}, conversion_options);
}
+void CheckInvalidEmitIndex(const std::string& rel_json) {
+ std::string substrait_json = R"({
+ "version": { "major_number": 9999, "minor_number": 9999, "patch_number":
9999 },
+ "relations": [{
+ "rel": )" + rel_json + R"(
+ }]
+ })";
+
+ ASSERT_OK_AND_ASSIGN(auto buf,
+ internal::SubstraitFromJSON("Plan", substrait_json,
+
/*ignore_unknown_fields=*/false));
+ auto dummy_schema =
+ schema({field("A", int32()), field("B", int32()), field("C", int32())});
+ ConversionOptions conversion_options;
+ conversion_options.named_table_provider =
+ AlwaysProvideSameTable(TableFromJSON(dummy_schema, {"[[1, 2, 3]]"}));
+ std::shared_ptr<ExtensionIdRegistry> sp_ext_id_reg =
MakeExtensionIdRegistry();
+ ExtensionIdRegistry* ext_id_reg = sp_ext_id_reg.get();
+ ExtensionSet ext_set(ext_id_reg);
+ EXPECT_THAT(
+ DeserializePlans(
+ *buf, [] { return kNullConsumer; }, ext_id_reg, &ext_set,
conversion_options),
+ Raises(StatusCode::Invalid, HasSubstr("Out of bounds emit index")));
+}
+
+constexpr std::string_view kEmitTestReadRel = R"({
+ "read": {
+ "base_schema": {
+ "names": ["A", "B", "C"],
+ "struct": {"types": [{"i32": {}}, {"i32": {}}, {"i32": {}}]}
+ },
+ "namedTable": {"names": ["A"]}
+ }
+})";
+
+TEST(Substrait, ReadRelWithOutOfBoundsEmit) {
+ for (const char* mapping : {"[0, 3]", "[-1, 1]"}) {
+ ARROW_SCOPED_TRACE("outputMapping: ", mapping);
+ CheckInvalidEmitIndex(R"({
+ "read": {
+ "common": {"emit": {"outputMapping": )" +
+ std::string(mapping) + R"(}},
+ "base_schema": {
+ "names": ["A", "B", "C"],
+ "struct": {"types": [{"i32": {}}, {"i32": {}}, {"i32": {}}]}
+ },
+ "namedTable": {"names": ["A"]}
+ }
+ })");
+ }
+}
+
+TEST(Substrait, ProjectRelWithOutOfBoundsEmit) {
+ // The project emits the 3 input fields plus 1 expression, so 4 is out of
range
+ for (const char* mapping : {"[0, 4]", "[-1, 3]"}) {
+ ARROW_SCOPED_TRACE("outputMapping: ", mapping);
+ CheckInvalidEmitIndex(R"({
+ "project": {
+ "common": {"emit": {"outputMapping": )" +
+ std::string(mapping) + R"(}},
+ "expressions": [
+ {"selection": {"directReference": {"structField": {"field": 0}},
+ "rootReference": {}}}
+ ],
+ "input": )" + std::string(kEmitTestReadRel) +
Review Comment:
Makes sense, I've inlined it into ProjectRelWithOutOfBoundsEmit so the 3
input fields the mappings are counted against sit right next to them. The
substrait test still passes locally in a debug build.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
