Repository navigation
Fix review findings in parsing, SQL, JSON and fulusproto - #15
Merged
Merged
Conversation
ParseRate and ParseFactor used big.Rat.SetString, which accepts base prefixes and underscores. So "010/100" gave 8/100 because "010" is octal, and "0x10" gave 16. A rate from a config file could load as a wrong value without an error. Both functions now accept only base 10 digits, an optional sign, and one decimal point or one fraction bar. Signed-off-by: ayn <ayn.khatib@gmail.com> Signed-off-by: ayn <ayn.khatib@gmail.com>
Money.Scan read an int64 as major units. Before the NUMERIC change it read an int64 as minor units, so a BIGINT column of cents that still uses Money loaded as 100 times the amount without an error. An int64 can be major units from NUMERIC or minor units from BIGINT, so Money and AnyMoney.ScanColumns now return ErrInvalidAmountFormat for it. BigintMoney reads minor units from an integer column. Signed-off-by: ayn <ayn.khatib@gmail.com> Signed-off-by: ayn <ayn.khatib@gmail.com>
A NUMERIC(19, 4) column gives "10.5000" for USD 10.50, and Scan failed with ErrScaleMismatch on each row. Zeros after the minor units do not change the value, so ParseMoney, UnmarshalJSON, Scan and format.Parse now accept them. A digit that is not zero still gives ErrScaleMismatch. Signed-off-by: ayn <ayn.khatib@gmail.com> Signed-off-by: ayn <ayn.khatib@gmail.com>
ParseMoney is documented to read the canonical decimal, but it also accepted a "+" sign and leading zeros such as "0010.50". It now returns ErrInvalidAmountFormat for both. The same rule applies to JSON, text, Scan and BigintMoney text. format.Parse still accepts leading zeros, because it reads amounts that a person wrote. Signed-off-by: ayn <ayn.khatib@gmail.com> Signed-off-by: ayn <ayn.khatib@gmail.com>
The zero AnyMoney gave {"amount":"0","currency":""}, and UnmarshalJSON
rejected it with ErrUnknownCurrency, so a struct with an unset AnyMoney
field did not round trip. MarshalJSON now writes null for the zero
value, and returns ErrUnknownCurrency for an amount without a currency.
Signed-off-by: ayn <ayn.khatib@gmail.com>
Signed-off-by: ayn <ayn.khatib@gmail.com>
FromMoney compared the currency code with case, but Money JSON and the currency registry do not. It now uses the same rule as they do. ToMoney parsed the units without the sign, so -9223372036854775808 gave ErrOverflow although it fits in int64. It now parses the sign with the units. Signed-off-by: ayn <ayn.khatib@gmail.com> Signed-off-by: ayn <ayn.khatib@gmail.com>
fulusproto/go.mod required github.com/khatibomar/fulus v0.0.0 with a replace directive to the parent directory. Go ignores the replace directives of a dependency, so a user could not resolve v0.0.0. fulusproto now requires v1.0.0-beta.5, the next release, and a go.work file lets the modules in this repository use the local fulus code. CONTRIBUTING tells how to release fulusproto after a fulus tag. Signed-off-by: ayn <ayn.khatib@gmail.com> Signed-off-by: ayn <ayn.khatib@gmail.com>
The README was 17 KB. It now keeps the reasons, the first examples, the features, the comparison, the performance and the stability policy, and links to docs/usage.md for the examples of each part of the API. Signed-off-by: ayn <ayn.khatib@gmail.com> Signed-off-by: ayn <ayn.khatib@gmail.com>
khatibomar
force-pushed
the
fix-review-findings
branch
from
October 10, 2026 02:08
a794fd5 to
9084894
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Parse rates and factors only in base 10, reject int64 in Money.Scan, accept zeros after the minor units, and accept only canonical decimals in ParseMoney.
Write null for the zero AnyMoney, fix the code case and MinInt64 units in fulusproto, and make fulusproto installable with go.work.
Tag v1.0.0-beta.5 before tagging fulusproto, because fulusproto/go.mod requires it.