Repository: planetlabs/gpq (go)

Issue:
Unable to convert Parquet files with WKT geometry
As reported in https://github.com/planetlabs/gpq/issues/33#issuecomment-1600016255, converting Parquet files with a geometry column containing WKT values does not work (when there is more than one row).

Pull request description:
Fix issue converting Parquet with WKT geometry
Previously, when converting an input Parquet file to a GeoParquet file, the same metadata struct would be used to read the input metadata and write the output metadata.  When the input geometry type is WKT, the output is converted to WKB.  During writing, the metadata was modified after writing the first geometry.  This meant that reading the second geometry failed since it was not WKB (but the modified metadata indicated that it was).

This change ensures that the input metadata is cloned before creating modified output metadata.

Fixes #34.

Patch:
diff --git a/internal/geoparquet/geoparquet.go b/internal/geoparquet/geoparquet.go
index f37699b..66d582d 100644
--- a/internal/geoparquet/geoparquet.go
+++ b/internal/geoparquet/geoparquet.go
@@ -61,6 +61,16 @@ type Metadata struct {
 	Columns       map[string]*GeometryColumn `json:"columns"`
 }
 
+func (m *Metadata) Clone() *Metadata {
+	clone := &Metadata{}
+	*clone = *m
+	clone.Columns = make(map[string]*GeometryColumn, len(m.Columns))
+	for i, v := range m.Columns {
+		clone.Columns[i] = v.clone()
+	}
+	return clone
+}
+
 type GeometryColumn struct {
 	Encoding      string    `json:"encoding"`
 	GeometryType  any       `json:"geometry_type,omitempty"`
@@ -72,6 +82,14 @@ type GeometryColumn struct {
 	Epoch         float64   `json:"epoch,omitempty"`
 }
 
+func (g *GeometryColumn) clone() *GeometryColumn {
+	clone := &GeometryColumn{}
+	*clone = *g
+	clone.Bounds = make([]float64, len(g.Bounds))
+	copy(clone.Bounds, g.Bounds)
+	return clone
+}
+
 func (col *GeometryColumn) GetGeometryTypes() []string {
 	if multiType, ok := col.GeometryTypes.([]any); ok {
 		types := make([]string, len(multiType))
@@ -347,15 +365,16 @@ func FromParquet(file *parquet.File, output io.Writer, convertOptions *ConvertOp
 	boundsLookup := map[string]*orb.Bound{}
 	geometryTypeLookup := map[string]map[string]bool{}
 
-	metadata, metadataErr := GetMetadata(file)
+	inputMetadata, metadataErr := GetMetadata(file)
 	if metadataErr != nil {
-		metadata = &Metadata{
+		inputMetadata = &Metadata{
 			PrimaryColumn: defaultGeometryColumn,
 			Columns: map[string]*GeometryColumn{
 				defaultGeometryColumn: {},
 			},
 		}
 	}
+	outputMetadata := inputMetadata.Clone()
 
 	for {
 		row, err := reader.Next()
@@ -371,12 +390,12 @@ func FromParquet(file *parquet.File, output io.Writer, convertOptions *ConvertOp
 			return err
 		}
 
-		for name, columnMetadata := range metadata.Columns {
+		for name, inputColumn := range inputMetadata.Columns {
 			value, ok := properties[name]
 			if !ok {
 				return fmt.Errorf("missing geometry column: %s", name)
 			}
-			geometry, encoding, err := Geometry(value, name, metadata, schema)
+			geometry, encoding, err := Geometry(value, name, inputMetadata, schema)
 			if err != nil {
 				return err
 			}
@@ -393,8 +412,8 @@ func FromParquet(file *parquet.File, output io.Writer, convertOptions *ConvertOp
 				row[column.ColumnIndex] = parquet.ValueOf(geomBytes)
 			}
 
-			if columnMetadata.Encoding != EncodingWKB {
-				columnMetadata.Encoding = EncodingWKB
+			if inputColumn.Encoding != EncodingWKB {
+				outputMetadata.Columns[name].Encoding = EncodingWKB
 			}
 
 			bounds := geometry.Bound()
@@ -417,10 +436,10 @@ func FromParquet(file *parquet.File, output io.Writer, convertOptions *ConvertOp
 
 	for name, bounds := range boundsLookup {
 		if bounds != nil {
-			if metadata.Columns[name] == nil {
-				metadata.Columns[name] = getDefaultGeometryColumn()
+			if inputMetadata.Columns[name] == nil {
+				outputMetadata.Columns[name] = getDefaultGeometryColumn()
 			}
-			metadata.Columns[name].Bounds = []float64{
+			outputMetadata.Columns[name].Bounds = []float64{
 				bounds.Left(), bounds.Bottom(), bounds.Right(), bounds.Top(),
 			}
 		}
@@ -433,13 +452,13 @@ func FromParquet(file *parquet.File, output io.Writer, convertOptions *ConvertOp
 				geometryTypes = append(geometryTypes, geometryType)
 			}
 		}
-		if metadata.Columns[name] == nil {
-			metadata.Columns[name] = getDefaultGeometryColumn()
+		if inputMetadata.Columns[name] == nil {
+			outputMetadata.Columns[name] = getDefaultGeometryColumn()
 		}
-		metadata.Columns[name].GeometryTypes = geometryTypes
+		outputMetadata.Columns[name].GeometryTypes = geometryTypes
 	}
 
-	metadataBytes, jsonErr := json.Marshal(metadata)
+	metadataBytes, jsonErr := json.Marshal(outputMetadata)
 	if jsonErr != nil {
 		return fmt.Errorf("failed to serialize geo metadata: %w", jsonErr)
 	}
diff --git a/readme.md b/readme.md
index 5ffa55f..99c4877 100644
--- a/readme.md
+++ b/readme.md
@@ -60,6 +60,15 @@ gpq convert example.geojson example.parquet
 gpq convert example.parquet example.geojson
 ```
 
+The `convert` command can also be used to convert an input Parquet file without "geo" metadata to a valid GeoParquet file.
+
+```shell
+# read parquet and write geoparquet
+gpq convert non-geo.parquet valid-geo.parquet
+```
+
+When reading from a Parquet file and writing out GeoParquet, the input geometry values can be WKB or WKT encoded.  The output geometry values will always be WKB encoded.
+
 The `--compression` argument can be used to control the compression codec used when writing GeoParquet.  See `gpq convert --help` for the available options.
 
 


Test patch:
diff --git a/internal/geoparquet/geoparquet_test.go b/internal/geoparquet/geoparquet_test.go
index 8b33981..974362f 100644
--- a/internal/geoparquet/geoparquet_test.go
+++ b/internal/geoparquet/geoparquet_test.go
@@ -232,6 +232,27 @@ func TestFromParquetWithoutMetadata(t *testing.T) {
 	assert.Equal(t, int64(1), geoparquetFile.NumRows())
 }
 
+func TestMetadataClone(t *testing.T) {
+	metadata := geoparquet.DefaultMetadata()
+	clone := metadata.Clone()
+
+	assert.Equal(t, metadata.PrimaryColumn, clone.PrimaryColumn)
+	clone.PrimaryColumn = "modified"
+	assert.NotEqual(t, metadata.PrimaryColumn, clone.PrimaryColumn)
+
+	assert.Equal(t, len(metadata.Columns), len(clone.Columns))
+
+	require.Contains(t, metadata.Columns, metadata.PrimaryColumn)
+	require.Contains(t, clone.Columns, metadata.PrimaryColumn)
+
+	originalColumn := metadata.Columns[metadata.PrimaryColumn]
+	cloneColumn := clone.Columns[metadata.PrimaryColumn]
+
+	assert.Equal(t, originalColumn.Encoding, cloneColumn.Encoding)
+	cloneColumn.Encoding = "modified"
+	assert.NotEqual(t, originalColumn.Encoding, cloneColumn.Encoding)
+}
+
 func TestFromParquetWithWKT(t *testing.T) {
 	type Row struct {
 		Name     string `parquet:"name"`
@@ -240,9 +261,13 @@ func TestFromParquetWithWKT(t *testing.T) {
 
 	rows := []*Row{
 		{
-			Name:     "test-point",
+			Name:     "test-point-1",
 			Geometry: string(wkt.Marshal(orb.Point{1, 2})),
 		},
+		{
+			Name:     "test-point-2",
+			Geometry: string(wkt.Marshal(orb.Point{3, 4})),
+		},
 	}
 
 	parquetFile, inputErr := makeParquet(rows, nil)
@@ -268,9 +293,9 @@ func TestFromParquetWithWKT(t *testing.T) {
 	assert.Contains(t, geometryTypes, "Point")
 
 	bounds := primaryColumnMetadata.Bounds
-	assert.Equal(t, []float64{1, 2, 1, 2}, bounds)
+	assert.Equal(t, []float64{1, 2, 3, 4}, bounds)
 
 	assert.Equal(t, geoparquet.EncodingWKB, primaryColumnMetadata.Encoding)
 
-	assert.Equal(t, int64(1), geoparquetFile.NumRows())
+	assert.Equal(t, int64(2), geoparquetFile.NumRows())
 }