westonpace commented on code in PR #18086:
URL: https://github.com/apache/datafusion/pull/18086#discussion_r2440791752


##########
datafusion/substrait/src/logical_plan/producer/expr/literal.rs:
##########
@@ -94,6 +95,36 @@ pub(crate) fn to_substrait_literal(
             LiteralType::I64(*n as i64),
             UNSIGNED_INTEGER_TYPE_VARIATION_REF,
         ),
+        ScalarValue::Float16(Some(f)) => {
+            // Following rules from 
https://github.com/apache/arrow/blame/main/format/substrait/extension_types.yaml
+            // fp16 literals are encoded as user defined literals with
+            // a google.protobuf.UInt32Value message where the lower 16 bits 
are
+            // the fp16 value.
+            let type_anchor = 
producer.register_type(FLOAT_16_TYPE_NAME.to_string());
+
+            // Hmm, the spec says "lower 16 bits" but neglects to mention the 
endianness.
+            // Let's just use little-endian for now and update the spec.

Review Comment:
   Added link to https://github.com/apache/arrow/issues/47846



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to