builtins: add st_asgeojson for recordsets

[otan]

Release note (sql change): Add ST_AsGeoJSON for recordsets, putting row
contents into the properties field of a GeoJSON object.

[cockroach-teamcity]

This change is 

[sumeerbhola]

Reviewed 5 of 5 files at r1.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @otan)

---

pkg/sql/logictest/testdata/logic_test/geospatial, line 426 at r1 (raw file):

CREATE TABLE parse_test_geojson AS
  SELECT
    row_number() OVER (ORDER BY id) as id,

I didn't quite understand how this table is different from parse_test -- isn't it doing a one-to-one mapping of rows and generating an id using the ordering of ids in parse_test?

---

pkg/sql/logictest/testdata/logic_test/geospatial, line 433 at r1 (raw file):

query TTT
SELECT
  ST_AsGeoJSON(parse_test_geojson.*),

this one without the geomcolumnname parameter picks the "geom" column. If there was no column named "geom", what would it do?

---

pkg/sql/logictest/testdata/logic_test/geospatial, line 462 at r1 (raw file):

   },
   "properties": {
     "geom": {

Should geom be appearing in properties given that it has already been incorporated into geometry? The examples at https://postgis.net/docs/ST_AsGeoJSON.html suggest not.
Hmm, is this because we've selected geog so all the other columns are in properties, which includes geom?

---

pkg/sql/sem/builtins/geo_builtins.go, line 4235 at r1 (raw file):

		}
		if label == "" {
			label = fmt.Sprintf("f%d", i+1)

where is this default labeling specified (from a user's perspective)?

---

pkg/sql/sem/builtins/geo_builtins.go, line 4260 at r1 (raw file):

				}
				continue
			}

So empty geoColumn implies we pick the first one we can parse. Is that consistent with PostGIS?

[otan]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @sumeerbhola)

---

pkg/sql/logictest/testdata/logic_test/geospatial, line 426 at r1 (raw file):

Previously, sumeerbhola wrote…

I didn't quite understand how this table is different from parse_test -- isn't it doing a one-to-one mapping of rows and generating an id using the ordering of ids in parse_test?

i'll add a comment - parse test relies on row_id which changes betwen runs. i needed this to be a fixed number to get this to work.

---

pkg/sql/logictest/testdata/logic_test/geospatial, line 433 at r1 (raw file):

Previously, sumeerbhola wrote…

this one without the geomcolumnname parameter picks the "geom" column. If there was no column named "geom", what would it do?

pretend it's a NULL geometry

---

pkg/sql/logictest/testdata/logic_test/geospatial, line 462 at r1 (raw file):

Previously, sumeerbhola wrote…

Should geom be appearing in properties given that it has already been incorporated into geometry? The examples at https://postgis.net/docs/ST_AsGeoJSON.html suggest not.
Hmm, is this because we've selected geog so all the other columns are in properties, which includes geom?

yep that's it -- if there's more than one, the rest go in properties.

---

pkg/sql/sem/builtins/geo_builtins.go, line 4235 at r1 (raw file):

Previously, sumeerbhola wrote…

where is this default labeling specified (from a user's perspective)?

seems to be the default if the tuple isn't labelled in row_to_json / for tuple types if there are no labels.

---

pkg/sql/sem/builtins/geo_builtins.go, line 4260 at r1 (raw file):

Previously, sumeerbhola wrote…

So empty geoColumn implies we pick the first one we can parse. Is that consistent with PostGIS?

yep!

[sumeerbhola]

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @otan and @sumeerbhola)

---

pkg/sql/sem/builtins/geo_builtins.go, line 4235 at r1 (raw file):

Previously, otan (Oliver Tan) wrote…

seems to be the default if the tuple isn't labelled in row_to_json / for tuple types if there are no labels.

Can you add a code comment with this reference.

[otan]

bors r=sumeerbhola

[craig]

Build succeeded:

• Compile Build (Cockroach)