- Notifications
You must be signed in to change notification settings - Fork54
WIP: Add decimal to StringRules#440
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
github-actionsbot commentedNov 24, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
The latest Buf updates on your PR. Results from workflowBuf CI / buf (pull_request).
|
hudlow commentedNov 24, 2025
@bufdev Is it safe to assume that the goal is to align these rules to what could be represented in a SQL If so (and based on the desire to possibly extend to the full range of |
bufdev commentedNov 24, 2025
Yes
Probably not? I don't know, what do SQL databases generally accept? |
hudlow commentedNov 25, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
From what I can tell, normalization prior to conversion is typical. Unfortunately, I don't have access to the relevant sections of ISO 9075, but this is runnable against all the engines on sqlfiddle.com and they all normalize the values without losing any magnitudinal information:
Yeah, it's strange. Of course, one or more leading zeroes is a common signifier for octal; for example, ECMAScript and Go both interpret The inconsistencies here are an argument for rejecting numbers with leading zeroes on the grounds that someone might expect you to interpret them as octal and someone else might not. My own pet peeve in the numeric literal space isthe lack of overall limits on digit count. I think any spec that says if I give you an unbounded number of zeroes before or after the decimal place you have to parse it—or you have to select your own limit—is saliently deficient. As far as treating trailing zeroes after the decimal point as indicators of precision, as is done for empirical quantities in science... it certainly bothers me when software calls things significant digits that aren't, but I can't recall ever encountering a programming language that actually treats numeric literals according to the rules of significant digits (where trailing zeroes after the decimal point are significant and trailing zeroes in a number without a decimal point are insignificant). One reason for this is probably that you can't truncate digits at each step in a mathematical operation, so a programming language that followed the rules of significant digits would have to track the precision of numeric values in parallel to their magnitude. In a greenfield context, I wouldn't be opposed to validating the significant digits of numeric literals with a predefined precision, but I think the precedent in the SQL-adjacent space of decimal literals has everything to do with whether a value's magnitude can be expressed without respect to inferring anything about its precision from the literal representation. |
bufdev commentedNov 25, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Overly restrictive better than too loose to start if there's a choice - we can always loosen. Time to market is most important. |
hudlow commentedNov 25, 2025
I'd say any change in strictness for a validation rule is a breaking change. |
bufdev commentedNov 26, 2025
Loosening is fine. |
hudlow commentedNov 26, 2025
I took a swing at an implementation, but hoping someone (@timostamm?) can help fill in some gaps for me:
|
| (predefined).cel = { | ||
| id:"string.decimal.precision_minimum" | ||
| message:"precision must be at least 1" | ||
| expression:"!has(rules.decimal.precision) || rules.decimal.precision > 0" | ||
| }, | ||
| (predefined).cel = { | ||
| id:"string.decimal.scale_without_precision" | ||
| message:"if scale is set, precision must also be set" | ||
| expression:"!has(rules.decimal.scale) || has(rules.decimal.precision)" | ||
| }, | ||
| (predefined).cel = { | ||
| id:"string.decimal.scale_less_than_precision" | ||
| message:"scale must be less than precision" | ||
| expression:"!has(rules.decimal.precision) || !has(rules.decimal.scale) || rules.decimal.scale < rules.decimal.precision" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@timostamm Do we have precedent for validations that do not usethis and consequently could be checked at compile time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Whenthis,rule, andrules are declared, implementations set the appropriate CEL type. Seehttps://github.com/bufbuild/protovalidate-go/blob/v1.0.1/cache.go#L160-L162. When the implementation compiles the expression, it can raise type errors.
timostamm commentedNov 26, 2025
This repository defines the validation rules and provides atest suite for implementations.
As in "if scale is set, precision must also be set"? The rule |
| // // point, and at most four digits before the decimal point. | ||
| // // "1000", "100.1", and "100" are valid, whereas "10000" and "1.111" are not. | ||
| // string value = 1 [ | ||
| // (buf.validate.field).decimal.precision = 6, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
(buf.validate.field).decimal is not a field.
(buf.validate.field).string is fieldStringRules string = 14.(buf.validate.field).string.len is fielduint64 len 19.(buf.validate.field).string.decimal is fieldDecimalRules decimal = 36.
hudlow commentedNov 26, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Update:
@bufdev out of band, I think you expressed some doubt as to this as a string format. Some thoughts:
|
This would basically both deprecate usage of
google.type.Decimaland actually enforce valid values with Protovalidate.