kevinjqliu commented on code in PR #16798:
URL: https://github.com/apache/iceberg/pull/16798#discussion_r3510716742


##########
format/spec.md:
##########
@@ -268,7 +268,7 @@ Supported primitive types are defined in the table below. 
Primitive types added
 |                  | **`long`**         | 64-bit signed integers               
                                    |                                           
       |
 |                  | **`float`**        | [32-bit IEEE 
754](https://en.wikipedia.org/wiki/IEEE_754) floating point | Can promote to 
double                            |
 |                  | **`double`**       | [64-bit IEEE 
754](https://en.wikipedia.org/wiki/IEEE_754) floating point |                   
                               |
-|                  | **`decimal(P,S)`** | Fixed-point decimal; precision P, 
scale S                                | Scale is fixed, precision must be 38 
or less     |
+|                  | **`decimal(P, S)`** | Fixed-point decimal; precision P, 
scale S                                | Scale is fixed, precision must be 38 
or less     |

Review Comment:
   I feel like its a good idea to document the "canonical" format, just to 
avoid any confusion. its a nit change, and im happy to revert it. 
   
   With the "hide whitespace" option, this PR becomes 2 lines of change + an 
optional formatting improvement
   https://github.com/apache/iceberg/pull/16798/changes?w=1
   
   Let me know what you think, I'm happy to move forward either way 



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