EpsilonPrime commented on code in PR #15063:
URL: https://github.com/apache/arrow/pull/15063#discussion_r1055739573


##########
cpp/src/arrow/engine/substrait/extension_set.cc:
##########
@@ -706,11 +706,11 @@ class EnumParser {
 };
 
 enum class TemporalComponent { kUnspecified = 0, kYear, kMonth, kDay, kSecond 
};
-static std::vector<std::string> kTemporalComponentOptions = {"YEAR", "MONTH", 
"DAY",
-                                                             "SECOND"};
+static std::vector<std::string> kTemporalComponentOptions = {"UNSPECIFIED", 
"YEAR",
+                                                             "MONTH", "DAY", 
"SECOND"};

Review Comment:
   Temporal extraction (in function tDecodeTemporalExtractionMapping) uses the 
kUnspecified state to determine if the enum argument was not passed to the 
function (to provide a more readable error message).  One alternative I 
considered was defaulting the value to kYear but that doesn't preserve the 
existing behavior.  I could remove kUnspecified if the less specific error is 
acceptable (I will have to investigate to see what the wording will be).



-- 
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]

Reply via email to