rraulinio commented on code in PR #1418: URL: https://github.com/apache/iceberg-go/pull/1418#discussion_r3535457975
########## udf/types.go: ########## @@ -0,0 +1,360 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package udf + +import ( + "encoding/json" + "errors" + "fmt" + "strings" + "unicode" + + "github.com/apache/iceberg-go" +) + +// ErrInvalidUDFType is returned when a UDF parameter or return type +// does not conform to the UDF spec's Types section. +var ErrInvalidUDFType = errors.New("invalid UDF type") + +// Type represents a UDF parameter or return type as defined by the UDF +// spec. UDF types are Iceberg types without field IDs: primitives are +// encoded as plain strings (e.g. "int", "decimal(9,2)") and nested types +// (list, map, struct) as JSON objects carrying only the fields the UDF +// spec requires. +// +// String returns the canonical string representation used to build +// definition IDs (see CanonicalDefinitionID). +type Type interface { + fmt.Stringer + + Equals(Type) bool + + // canonicalTo writes the canonical string form, sealing the + // interface to the implementations in this package. + canonicalTo(sb *strings.Builder) +} + +// PrimitiveType is a primitive or semi-structured UDF type, stored as the +// verbatim type string (e.g. "int", "decimal(9,2)", "variant"). Values are +// always validated: construct them with PrimitiveTypeOf. +type PrimitiveType struct { + name string +} + +// PrimitiveTypeOf validates s against the Iceberg type system and the UDF +// spec's rules for type strings (no spaces or quote characters) and returns +// the corresponding PrimitiveType. +func PrimitiveTypeOf(s string) (PrimitiveType, error) { + if s == "" { + return PrimitiveType{}, fmt.Errorf("%w: type string must not be empty", ErrInvalidUDFType) + } + + if strings.ContainsFunc(s, unicode.IsSpace) || strings.ContainsAny(s, `"'`) { Review Comment: Thanks for checking this. I agree the table type renderers commonly use `decimal(9, 2)`, but the UDF metadata spec has a stricter serialization rule here. In the UDF spec's Types section, primitive/semi-structured type strings use examples like `decimal(9,2)` and it explicitly says: "Type strings must contain no spaces or quote characters." The Definition ID section also says the canonical string is formatted with no spaces: https://iceberg.apache.org/udf-spec/#types So for UDF metadata, `decimal(9, 2)` is invalid input per the UDF spec, even though it is a familiar table-schema rendering. Normalizing through iceberg-go's parsed `DecimalType.String()` would actually move this package in the wrong direction, because that returns the spaced table form and would make us write spec-invalid UDF type strings / definition-ids. I kept this strict and test-pinned so definition IDs stay on the UDF spec's canonical no-space form. -- 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]
