You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/10/13 13:34:13 UTC

[GitHub] [arrow] westonpace opened a new pull request, #14402: ARROW-15540: [C++] Allow the substrait consumer to accept plans with hints and nullable literals

westonpace opened a new pull request, #14402:
URL: https://github.com/apache/arrow/pull/14402

   As long as the conversion strictness is not EXACT_ROUNDTRIP we now ignore these fields if present.


-- 
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@arrow.apache.org

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


[GitHub] [arrow] nealrichardson merged pull request #14402: ARROW-15540: [C++] Allow the substrait consumer to accept plans with hints and nullable literals

Posted by GitBox <gi...@apache.org>.
nealrichardson merged PR #14402:
URL: https://github.com/apache/arrow/pull/14402


-- 
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@arrow.apache.org

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


[GitHub] [arrow] westonpace commented on a diff in pull request #14402: ARROW-15540: [C++] Allow the substrait consumer to accept plans with hints and nullable literals

Posted by GitBox <gi...@apache.org>.
westonpace commented on code in PR #14402:
URL: https://github.com/apache/arrow/pull/14402#discussion_r994926310


##########
cpp/src/arrow/engine/substrait/serde_test.cc:
##########
@@ -402,22 +402,36 @@ TEST(Substrait, SupportedLiterals) {
   auto ExpectEq = [](std::string_view json, Datum expected_value) {
     ARROW_SCOPED_TRACE(json);
 
-    ASSERT_OK_AND_ASSIGN(
-        auto buf, internal::SubstraitFromJSON("Expression",
-                                              "{\"literal\":" + std::string(json) + "}"));
-    ExtensionSet ext_set;
-    ASSERT_OK_AND_ASSIGN(auto expr, DeserializeExpression(*buf, ext_set));
+    for (bool nullable : {false, true}) {
+      std::string json_with_nullable;
+      std::string nullable_json = (nullable) ? ", \"nullable\": true" : "";
+      if (nullable) {
+        auto final_closing_brace = json.find_last_of('}');
+        ASSERT_NE(std::string_view::npos, final_closing_brace);
+        json_with_nullable =
+            std::string(json.substr(0, final_closing_brace)) + ", \"nullable\": true}";
+        json = json_with_nullable;
+      }

Review Comment:
   Whoops, yeah, `nullable_json` was an earlier attempt and should just be removed.  The point on ignoring unknown fields is a good one though.  I'll make a crack at 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@arrow.apache.org

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


[GitHub] [arrow] drin commented on a diff in pull request #14402: ARROW-15540: [C++] Allow the substrait consumer to accept plans with hints and nullable literals

Posted by GitBox <gi...@apache.org>.
drin commented on code in PR #14402:
URL: https://github.com/apache/arrow/pull/14402#discussion_r996015521


##########
cpp/src/arrow/engine/substrait/serde.h:
##########
@@ -261,10 +261,17 @@ Status CheckMessagesEquivalent(std::string_view message_name, const Buffer& l_bu
 ///
 /// \param[in] type_name the name of the Substrait message type to convert
 /// \param[in] json the JSON string to convert
+/// \param[in] ignore_unknown_fields if true then unknown fields will be ignored and
+///            will not cause an error
+///
+///            This should generally be true to allow consumption of plans from newer
+///            producers but setting to false can be useful if you are testing
+///            conformance to a specific Substrait version
 /// \return a buffer filled with the binary protobuf serialization of message
 ARROW_ENGINE_EXPORT
 Result<std::shared_ptr<Buffer>> SubstraitFromJSON(std::string_view type_name,
-                                                  std::string_view json);
+                                                  std::string_view json,
+                                                  bool ignore_unknown_fields = false);

Review Comment:
   Just my guess, but `outdated` might mean that the line numbers no longer match up (surrounding code has changed beyond some threshold). Since this change is within other context, the diff itself is not outdated.



-- 
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@arrow.apache.org

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


[GitHub] [arrow] westonpace commented on a diff in pull request #14402: ARROW-15540: [C++] Allow the substrait consumer to accept plans with hints and nullable literals

Posted by GitBox <gi...@apache.org>.
westonpace commented on code in PR #14402:
URL: https://github.com/apache/arrow/pull/14402#discussion_r995153605


##########
cpp/src/arrow/engine/substrait/serde_test.cc:
##########
@@ -402,22 +402,36 @@ TEST(Substrait, SupportedLiterals) {
   auto ExpectEq = [](std::string_view json, Datum expected_value) {
     ARROW_SCOPED_TRACE(json);
 
-    ASSERT_OK_AND_ASSIGN(
-        auto buf, internal::SubstraitFromJSON("Expression",
-                                              "{\"literal\":" + std::string(json) + "}"));
-    ExtensionSet ext_set;
-    ASSERT_OK_AND_ASSIGN(auto expr, DeserializeExpression(*buf, ext_set));
+    for (bool nullable : {false, true}) {
+      std::string json_with_nullable;
+      std::string nullable_json = (nullable) ? ", \"nullable\": true" : "";
+      if (nullable) {
+        auto final_closing_brace = json.find_last_of('}');
+        ASSERT_NE(std::string_view::npos, final_closing_brace);
+        json_with_nullable =
+            std::string(json.substr(0, final_closing_brace)) + ", \"nullable\": true}";
+        json = json_with_nullable;
+      }

Review Comment:
   I've added the bool argument to `SubstraitFromJSON` and set it to false for all unit test calls.  To my pleasant surprise, my addition was the only new failure.  I removed `nullable_json`.



-- 
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@arrow.apache.org

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


[GitHub] [arrow] westonpace commented on a diff in pull request #14402: ARROW-15540: [C++] Allow the substrait consumer to accept plans with hints and nullable literals

Posted by GitBox <gi...@apache.org>.
westonpace commented on code in PR #14402:
URL: https://github.com/apache/arrow/pull/14402#discussion_r996009050


##########
cpp/src/arrow/engine/substrait/serde.h:
##########
@@ -261,10 +261,17 @@ Status CheckMessagesEquivalent(std::string_view message_name, const Buffer& l_bu
 ///
 /// \param[in] type_name the name of the Substrait message type to convert
 /// \param[in] json the JSON string to convert
+/// \param[in] ignore_unknown_fields if true then unknown fields will be ignored and
+///            will not cause an error
+///
+///            This should generally be true to allow consumption of plans from newer
+///            producers but setting to false can be useful if you are testing
+///            conformance to a specific Substrait version
 /// \return a buffer filled with the binary protobuf serialization of message
 ARROW_ENGINE_EXPORT
 Result<std::shared_ptr<Buffer>> SubstraitFromJSON(std::string_view type_name,
-                                                  std::string_view json);
+                                                  std::string_view json,
+                                                  bool ignore_unknown_fields = false);

Review Comment:
   Hmm...I'm confused that Github is not marking this outdated.  The default was `false` but I already changed it to be `true` last night.



-- 
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@arrow.apache.org

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


[GitHub] [arrow] bkietz commented on a diff in pull request #14402: ARROW-15540: [C++] Allow the substrait consumer to accept plans with hints and nullable literals

Posted by GitBox <gi...@apache.org>.
bkietz commented on code in PR #14402:
URL: https://github.com/apache/arrow/pull/14402#discussion_r994898370


##########
cpp/src/arrow/engine/substrait/serde_test.cc:
##########
@@ -402,22 +402,36 @@ TEST(Substrait, SupportedLiterals) {
   auto ExpectEq = [](std::string_view json, Datum expected_value) {
     ARROW_SCOPED_TRACE(json);
 
-    ASSERT_OK_AND_ASSIGN(
-        auto buf, internal::SubstraitFromJSON("Expression",
-                                              "{\"literal\":" + std::string(json) + "}"));
-    ExtensionSet ext_set;
-    ASSERT_OK_AND_ASSIGN(auto expr, DeserializeExpression(*buf, ext_set));
+    for (bool nullable : {false, true}) {
+      std::string json_with_nullable;
+      std::string nullable_json = (nullable) ? ", \"nullable\": true" : "";
+      if (nullable) {
+        auto final_closing_brace = json.find_last_of('}');
+        ASSERT_NE(std::string_view::npos, final_closing_brace);
+        json_with_nullable =
+            std::string(json.substr(0, final_closing_brace)) + ", \"nullable\": true}";
+        json = json_with_nullable;
+      }

Review Comment:
   I see ignoring unknown fields was added in #13605 since the json conversion is being used in more places. This should probably be an argument to `SubstraitFromJSON` instead, so that we can be stricter while testing.



-- 
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@arrow.apache.org

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


[GitHub] [arrow] bkietz commented on a diff in pull request #14402: ARROW-15540: [C++] Allow the substrait consumer to accept plans with hints and nullable literals

Posted by GitBox <gi...@apache.org>.
bkietz commented on code in PR #14402:
URL: https://github.com/apache/arrow/pull/14402#discussion_r995788662


##########
cpp/src/arrow/engine/substrait/serde.h:
##########
@@ -261,10 +261,17 @@ Status CheckMessagesEquivalent(std::string_view message_name, const Buffer& l_bu
 ///
 /// \param[in] type_name the name of the Substrait message type to convert
 /// \param[in] json the JSON string to convert
+/// \param[in] ignore_unknown_fields if true then unknown fields will be ignored and
+///            will not cause an error
+///
+///            This should generally be true to allow consumption of plans from newer
+///            producers but setting to false can be useful if you are testing
+///            conformance to a specific Substrait version
 /// \return a buffer filled with the binary protobuf serialization of message
 ARROW_ENGINE_EXPORT
 Result<std::shared_ptr<Buffer>> SubstraitFromJSON(std::string_view type_name,
-                                                  std::string_view json);
+                                                  std::string_view json,
+                                                  bool ignore_unknown_fields = false);

Review Comment:
   I think the default should probably be `true`, since this is the current behavior expected by the bindings which use `SubstraitFromJSON` without the explicit argument (I believe this was the intent of https://github.com/apache/arrow/pull/13605 , @vibhatha ?). Otherwise if you feel like being fancy we could get the default from an environment variable...
   ```suggestion
                                                     bool ignore_unknown_fields = true);
   ```



-- 
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@arrow.apache.org

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


[GitHub] [arrow] bkietz commented on a diff in pull request #14402: ARROW-15540: [C++] Allow the substrait consumer to accept plans with hints and nullable literals

Posted by GitBox <gi...@apache.org>.
bkietz commented on code in PR #14402:
URL: https://github.com/apache/arrow/pull/14402#discussion_r994822479


##########
cpp/src/arrow/engine/substrait/expression_internal.cc:
##########
@@ -295,7 +295,8 @@ Result<compute::Expression> FromProto(const substrait::Expression& expr,
 Result<Datum> FromProto(const substrait::Expression::Literal& lit,
                         const ExtensionSet& ext_set,
                         const ConversionOptions& conversion_options) {
-  if (lit.nullable()) {

Review Comment:
   `Expression.Literal.{null, empty_list, empty_map}` are all `Type`s, which can independently express nullability. There's [a comment](https://github.com/substrait-io/substrait/blob/f3f6bdc947e689e800279666ff33f118e42d2146/proto/substrait/algebra.proto#L565-L567) for `Expression.Literal.null` which indicates that Literal.nullable should be left false since its type should carry that property. In light of this I'd propose we validate that if `lit` contains one of `Expression.Literal.{null, empty_list, empty_map}` then `lit.nullable()` must be false regardless of the conversion options



##########
cpp/src/arrow/engine/substrait/serde_test.cc:
##########
@@ -402,22 +402,36 @@ TEST(Substrait, SupportedLiterals) {
   auto ExpectEq = [](std::string_view json, Datum expected_value) {
     ARROW_SCOPED_TRACE(json);
 
-    ASSERT_OK_AND_ASSIGN(
-        auto buf, internal::SubstraitFromJSON("Expression",
-                                              "{\"literal\":" + std::string(json) + "}"));
-    ExtensionSet ext_set;
-    ASSERT_OK_AND_ASSIGN(auto expr, DeserializeExpression(*buf, ext_set));
+    for (bool nullable : {false, true}) {
+      std::string json_with_nullable;
+      std::string nullable_json = (nullable) ? ", \"nullable\": true" : "";
+      if (nullable) {
+        auto final_closing_brace = json.find_last_of('}');
+        ASSERT_NE(std::string_view::npos, final_closing_brace);
+        json_with_nullable =
+            std::string(json.substr(0, final_closing_brace)) + ", \"nullable\": true}";
+        json = json_with_nullable;
+      }

Review Comment:
   IIUC, this results in
   ```json
   {"literal": {"boolean": true, "nullable": true}, "nullable": true}
   ```
   but literal_type a `oneof` so it should only have a single key-value pair, where the key names the member of the union:
   ```json
   {"literal": {"boolean": true}, "nullable": true}
   ```
   If this isn't being caught in tests maybe there's a flag "error on unused fields" we should set.
   ```suggestion
   ```



##########
cpp/src/arrow/engine/substrait/serde_test.cc:
##########
@@ -504,6 +518,8 @@ TEST(Substrait, SupportedLiterals) {
     ASSERT_OK_AND_ASSIGN(auto json, internal::SubstraitToJSON("Type", *buf));
     ExpectEq("{\"null\": " + json + "}", MakeNullScalar(type));
   }
+
+  ConversionOptions conversion_options;

Review Comment:
   This seems unused, did you mean to put it in `ExpectEq` with 
   `conversion_options.strictness = ConversionStrictness::BEST_EFFORT`?



-- 
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@arrow.apache.org

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


[GitHub] [arrow] westonpace commented on a diff in pull request #14402: ARROW-15540: [C++] Allow the substrait consumer to accept plans with hints and nullable literals

Posted by GitBox <gi...@apache.org>.
westonpace commented on code in PR #14402:
URL: https://github.com/apache/arrow/pull/14402#discussion_r995162119


##########
cpp/src/arrow/engine/substrait/expression_internal.cc:
##########
@@ -295,7 +295,8 @@ Result<compute::Expression> FromProto(const substrait::Expression& expr,
 Result<Datum> FromProto(const substrait::Expression::Literal& lit,
                         const ExtensionSet& ext_set,
                         const ConversionOptions& conversion_options) {
-  if (lit.nullable()) {

Review Comment:
   Also, for a list, the type-nullability applies to the values in the list and not the lists themselves.  Same goes for maps.  The key/value nullability is distinct from whether you can have an entirely null map.
   
   So this just leaves the null case and it is very weird to expect producers to mark a null literal as non-nullable.



-- 
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@arrow.apache.org

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


[GitHub] [arrow] bkietz commented on a diff in pull request #14402: ARROW-15540: [C++] Allow the substrait consumer to accept plans with hints and nullable literals

Posted by GitBox <gi...@apache.org>.
bkietz commented on code in PR #14402:
URL: https://github.com/apache/arrow/pull/14402#discussion_r995788662


##########
cpp/src/arrow/engine/substrait/serde.h:
##########
@@ -261,10 +261,17 @@ Status CheckMessagesEquivalent(std::string_view message_name, const Buffer& l_bu
 ///
 /// \param[in] type_name the name of the Substrait message type to convert
 /// \param[in] json the JSON string to convert
+/// \param[in] ignore_unknown_fields if true then unknown fields will be ignored and
+///            will not cause an error
+///
+///            This should generally be true to allow consumption of plans from newer
+///            producers but setting to false can be useful if you are testing
+///            conformance to a specific Substrait version
 /// \return a buffer filled with the binary protobuf serialization of message
 ARROW_ENGINE_EXPORT
 Result<std::shared_ptr<Buffer>> SubstraitFromJSON(std::string_view type_name,
-                                                  std::string_view json);
+                                                  std::string_view json,
+                                                  bool ignore_unknown_fields = false);

Review Comment:
   I think the default should probably be `true`, since this is the current behavior expected by the bindings which use `SubstraitFromJSON` without the explicit argument. Otherwise if you feel like being fancy we could get the default from an environment variable...
   ```suggestion
                                                     bool ignore_unknown_fields = true);
   ```



-- 
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@arrow.apache.org

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


[GitHub] [arrow] bkietz commented on a diff in pull request #14402: ARROW-15540: [C++] Allow the substrait consumer to accept plans with hints and nullable literals

Posted by GitBox <gi...@apache.org>.
bkietz commented on code in PR #14402:
URL: https://github.com/apache/arrow/pull/14402#discussion_r996011074


##########
cpp/src/arrow/engine/substrait/serde.h:
##########
@@ -261,10 +261,17 @@ Status CheckMessagesEquivalent(std::string_view message_name, const Buffer& l_bu
 ///
 /// \param[in] type_name the name of the Substrait message type to convert
 /// \param[in] json the JSON string to convert
+/// \param[in] ignore_unknown_fields if true then unknown fields will be ignored and
+///            will not cause an error
+///
+///            This should generally be true to allow consumption of plans from newer
+///            producers but setting to false can be useful if you are testing
+///            conformance to a specific Substrait version
 /// \return a buffer filled with the binary protobuf serialization of message
 ARROW_ENGINE_EXPORT
 Result<std::shared_ptr<Buffer>> SubstraitFromJSON(std::string_view type_name,
-                                                  std::string_view json);
+                                                  std::string_view json,
+                                                  bool ignore_unknown_fields = false);

Review Comment:
   huh, odd



-- 
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@arrow.apache.org

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


[GitHub] [arrow] github-actions[bot] commented on pull request #14402: ARROW-15540: [C++] Allow the substrait consumer to accept plans with hints and nullable literals

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14402:
URL: https://github.com/apache/arrow/pull/14402#issuecomment-1278057254

   https://issues.apache.org/jira/browse/ARROW-15540


-- 
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@arrow.apache.org

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


[GitHub] [arrow] ursabot commented on pull request #14402: ARROW-15540: [C++] Allow the substrait consumer to accept plans with hints and nullable literals

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #14402:
URL: https://github.com/apache/arrow/pull/14402#issuecomment-1290975101

   ['Python', 'R'] benchmarks have high level of regressions.
   [test-mac-arm](https://conbench.ursa.dev/compare/runs/f6c788e8e2144414a837dd2da1f439d0...6d78dc9be9c14dc5a238db119bd55fb4/)
   [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/c05bb515a51f46009e28f062e5829be4...ff3508b78a864b7994acaad7cb0f0bd6/)
   


-- 
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@arrow.apache.org

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


[GitHub] [arrow] github-actions[bot] commented on pull request #14402: ARROW-15540: [C++] Allow the substrait consumer to accept plans with hints and nullable literals

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14402:
URL: https://github.com/apache/arrow/pull/14402#issuecomment-1278057369

   :warning: Ticket **has not been started in JIRA**, please click 'Start Progress'.


-- 
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@arrow.apache.org

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


[GitHub] [arrow] westonpace commented on a diff in pull request #14402: ARROW-15540: [C++] Allow the substrait consumer to accept plans with hints and nullable literals

Posted by GitBox <gi...@apache.org>.
westonpace commented on code in PR #14402:
URL: https://github.com/apache/arrow/pull/14402#discussion_r995153934


##########
cpp/src/arrow/engine/substrait/serde_test.cc:
##########
@@ -504,6 +518,8 @@ TEST(Substrait, SupportedLiterals) {
     ASSERT_OK_AND_ASSIGN(auto json, internal::SubstraitToJSON("Type", *buf));
     ExpectEq("{\"null\": " + json + "}", MakeNullScalar(type));
   }
+
+  ConversionOptions conversion_options;

Review Comment:
   Nope, this is just leftover from an earlier iteration where I was testing nullability outside of `ExpectEq`.  Thanks for catching.  I've removed 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@arrow.apache.org

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


[GitHub] [arrow] westonpace commented on a diff in pull request #14402: ARROW-15540: [C++] Allow the substrait consumer to accept plans with hints and nullable literals

Posted by GitBox <gi...@apache.org>.
westonpace commented on code in PR #14402:
URL: https://github.com/apache/arrow/pull/14402#discussion_r995161345


##########
cpp/src/arrow/engine/substrait/expression_internal.cc:
##########
@@ -295,7 +295,8 @@ Result<compute::Expression> FromProto(const substrait::Expression& expr,
 Result<Datum> FromProto(const substrait::Expression::Literal& lit,
                         const ExtensionSet& ext_set,
                         const ConversionOptions& conversion_options) {
-  if (lit.nullable()) {

Review Comment:
   I don't think that comment says it must be set to false.  It simply says the field does not apply.
   
   I'm not sure I support being pedantic here for no reason as we are not a Substrait validator.  In general, if an option doesn't make sense, but we can safely ignore it, I think we should.



-- 
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@arrow.apache.org

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


[GitHub] [arrow] ursabot commented on pull request #14402: ARROW-15540: [C++] Allow the substrait consumer to accept plans with hints and nullable literals

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #14402:
URL: https://github.com/apache/arrow/pull/14402#issuecomment-1290974490

   Benchmark runs are scheduled for baseline = 8b8841d4d77cfe43970904cc145e88936dca287b and contender = f5e592eb5ea89ae625edb2613a430374fb28a31c. f5e592eb5ea89ae625edb2613a430374fb28a31c is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/502a2911fe464eb2b21442bedf173eaa...f2f98805ce7a46a3be19eee7a544ed2a/)
   [Failed :arrow_down:0.0% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/f6c788e8e2144414a837dd2da1f439d0...6d78dc9be9c14dc5a238db119bd55fb4/)
   [Finished :arrow_down:0.0% :arrow_up:0.27%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/c05bb515a51f46009e28f062e5829be4...ff3508b78a864b7994acaad7cb0f0bd6/)
   [Failed] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/5881dfba582e42a48d52f300164cb222...ec7c5a8527454cd9b7c70e5719a20d8c/)
   Buildkite builds:
   [Finished] [`f5e592eb` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1723)
   [Failed] [`f5e592eb` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1742)
   [Finished] [`f5e592eb` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1725)
   [Failed] [`f5e592eb` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1737)
   [Finished] [`8b8841d4` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1722)
   [Failed] [`8b8841d4` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1741)
   [Finished] [`8b8841d4` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1724)
   [Finished] [`8b8841d4` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1735)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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@arrow.apache.org

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