[pocketbase#3110] normalized view queries with numeric or expression ids

.github/workflows/release.yaml

@@ -21,7 +21,7 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@v3
         with:
-          go-version: '>=1.20.3'
+          go-version: '>=1.21.0'
 
       # This step usually is not needed because the /ui/dist is pregenerated locally
       # but its here to ensure that each release embeds the latest admin ui artifacts.

CHANGELOG.md

@@ -1,11 +1,16 @@
-## v0.17.4-WIP
+## v0.17.4
+
+- Fixed Views record retrieval when numeric id is used ([#3110](https://github.com/pocketbase/pocketbase/issues/3110)).
+  _With this fix we also now properly recognize `CAST(... as TEXT)` and `CAST(... as BOOLEAN)` as `text` and `bool` fields._
 
 - Fixed `relation` "Cascade delete" tooltip message ([#3098](https://github.com/pocketbase/pocketbase/issues/3098)).
 
 - Fixed jsvm error message prefix on failed migrations ([#3103](https://github.com/pocketbase/pocketbase/pull/3103); thanks @nzhenev).
 
 - Disabled the initial Admin UI admins counter cache when there are no initial admins to allow detecting externally created accounts (eg. with the `admin` command) ([#3106](https://github.com/pocketbase/pocketbase/issues/3106)).
 
+- Downgraded `google/go-cloud` dependency to v0.32.0 until v0.34.0 is released to prevent the `os.TempDir` `cross-device link` errors as too many users complained about it.
+
 
 ## v0.17.3
 

apis/collection_test.go

@@ -47,7 +47,7 @@ func TestCollectionsList(t *testing.T) {
 			ExpectedContent: []string{
 				`"page":1`,
 				`"perPage":30`,
-				`"totalItems":10`,
+				`"totalItems":11`,
 				`"items":[{`,
 				`"id":"_pb_users_auth_"`,
 				`"id":"v851q4r790rhknl"`,
@@ -57,6 +57,7 @@ func TestCollectionsList(t *testing.T) {
 				`"id":"wzlqyes4orhoygb"`,
 				`"id":"4d1blo5cuycfaca"`,
 				`"id":"9n89pl5vkct6330"`,
+				`"id":"ib3m2700k5hlsjz"`,
 				`"type":"auth"`,
 				`"type":"base"`,
 			},
@@ -75,9 +76,9 @@ func TestCollectionsList(t *testing.T) {
 			ExpectedContent: []string{
 				`"page":2`,
 				`"perPage":2`,
-				`"totalItems":10`,
+				`"totalItems":11`,
 				`"items":[{`,
-				`"id":"kpv709sk2lqbqk8"`,
+				`"id":"v9gwnfh02gjq1q0"`,
 				`"id":"9n89pl5vkct6330"`,
 			},
 			ExpectedEvents: map[string]int{
@@ -1164,7 +1165,7 @@ func TestCollectionUpdate(t *testing.T) {
 }
 
 func TestCollectionsImport(t *testing.T) {
-	totalCollections := 10
+	totalCollections := 11
 
 	scenarios := []tests.ApiScenario{
 		{
@@ -1421,8 +1422,8 @@ func TestCollectionsImport(t *testing.T) {
 			ExpectedEvents: map[string]int{
 				"OnCollectionsAfterImportRequest":  1,
 				"OnCollectionsBeforeImportRequest": 1,
-				"OnModelBeforeDelete":              8,
-				"OnModelAfterDelete":               8,
+				"OnModelBeforeDelete":              9,
+				"OnModelAfterDelete":               9,
 				"OnModelBeforeUpdate":              2,
 				"OnModelAfterUpdate":               2,
 				"OnModelBeforeCreate":              1,

apis/record_crud_test.go

@@ -462,6 +462,22 @@ func TestRecordCrudList(t *testing.T) {
 			},
 			ExpectedEvents: map[string]int{"OnRecordsListRequest": 1},
 		},
+		{
+			Name:           "view collection with numeric ids",
+			Method:         http.MethodGet,
+			Url:            "/api/collections/numeric_id_view/records",
+			ExpectedStatus: 200,
+			ExpectedContent: []string{
+				`"page":1`,
+				`"perPage":30`,
+				`"totalPages":1`,
+				`"totalItems":2`,
+				`"items":[{`,
+				`"id":"1"`,
+				`"id":"2"`,
+			},
+			ExpectedEvents: map[string]int{"OnRecordsListRequest": 1},
+		},
 	}
 
 	for _, scenario := range scenarios {
@@ -731,6 +747,16 @@ func TestRecordCrudView(t *testing.T) {
 			},
 			ExpectedEvents: map[string]int{"OnRecordViewRequest": 1},
 		},
+		{
+			Name:           "view record with numeric id",
+			Method:         http.MethodGet,
+			Url:            "/api/collections/numeric_id_view/records/1",
+			ExpectedStatus: 200,
+			ExpectedContent: []string{
+				`"id":"1"`,
+			},
+			ExpectedEvents: map[string]int{"OnRecordViewRequest": 1},
+		},
 	}
 
 	for _, scenario := range scenarios {

daos/collection.go

@@ -378,6 +378,12 @@ func (dao *Dao) saveViewCollection(newCollection, oldCollection *models.Collecti
 			}
 		}
 
+		// wrap view query if necessary
+		query, err = txDao.normalizeViewQueryId(query)
+		if err != nil {
+			return fmt.Errorf("failed to normalize view query id: %w", err)
+		}
+
 		// (re)create the view
 		if err := txDao.SaveView(newCollection.Name, query); err != nil {
 			return err
@@ -389,6 +395,52 @@ func (dao *Dao) saveViewCollection(newCollection, oldCollection *models.Collecti
 	})
 }
 
+// @todo consider removing once custom id types are supported
+//
+// normalizeViewQueryId wraps (if necessary) the provided view query
+// with a subselect to ensure that the id column is a text since
+// currently we don't support non-string model ids
+// (see https://github.com/pocketbase/pocketbase/issues/3110).
+func (dao *Dao) normalizeViewQueryId(query string) (string, error) {
+	parsed, err := dao.parseQueryToFields(query)
+	if err != nil {
+		return "", err
+	}
+
+	needWrapping := true
+
+	idField := parsed[schema.FieldNameId]
+	if idField != nil && idField.field != nil &&
+		idField.field.Type != schema.FieldTypeJson &&
+		idField.field.Type != schema.FieldTypeNumber &&
+		idField.field.Type != schema.FieldTypeBool {
+		needWrapping = false
+	}
+
+	if !needWrapping {
+		return query, nil // no changes needed
+	}
+
+	// raw parse to preserve the columns order
+	rawParsed := new(identifiersParser)
+	if err := rawParsed.parse(query); err != nil {
+		return "", err
+	}
+
+	columns := make([]string, 0, len(rawParsed.columns))
+	for _, col := range rawParsed.columns {
+		if col.alias == schema.FieldNameId {
+			columns = append(columns, fmt.Sprintf("cast(%s as text) %s", col.alias, col.alias))
+		} else {
+			columns = append(columns, col.alias)
+		}
+	}
+
+	query = fmt.Sprintf("SELECT %s FROM (%s)", strings.Join(columns, ","), query)
+
+	return query, nil
+}
+
 // resaveViewsWithChangedSchema updates all view collections with changed schemas.
 func (dao *Dao) resaveViewsWithChangedSchema(excludeIds ...string) error {
 	collections, err := dao.FindCollectionsByType(models.CollectionTypeView)

daos/collection_test.go

@@ -210,11 +210,12 @@ func TestDeleteCollection(t *testing.T) {
 		{colUnsaved, true},
 		{colReferenced, true},
 		{colSystem, true},
+		{colBase, true},  // depend on view1, view2 and view2
 		{colView1, true}, // view2 depend on it
 		{colView2, false},
 		{colView1, false}, // no longer has dependent collections
-		{colBase, false},
-		{colAuth, false}, // should delete also its related external auths
+		{colBase, false},  // no longer has dependent views
+		{colAuth, false},  // should delete also its related external auths
 	}
 
 	for i, s := range scenarios {
@@ -393,8 +394,117 @@ func TestSaveCollectionIndirectViewsUpdate(t *testing.T) {
 	}
 }
 
+func TestSaveCollectionViewWrapping(t *testing.T) {
+	viewName := "test_wrapping"
+
+	scenarios := []struct {
+		name     string
+		query    string
+		expected string
+	}{
+		{
+			"no wrapping - text field",
+			"select text as id, bool from demo1",
+			"CREATE VIEW `test_wrapping` AS SELECT * FROM (select text as id, bool from demo1)",
+		},
+		{
+			"no wrapping - id field",
+			"select text as id, bool from demo1",
+			"CREATE VIEW `test_wrapping` AS SELECT * FROM (select text as id, bool from demo1)",
+		},
+		{
+			"no wrapping - relation field",
+			"select rel_one as id, bool from demo1",
+			"CREATE VIEW `test_wrapping` AS SELECT * FROM (select rel_one as id, bool from demo1)",
+		},
+		{
+			"no wrapping - select field",
+			"select select_many as id, bool from demo1",
+			"CREATE VIEW `test_wrapping` AS SELECT * FROM (select select_many as id, bool from demo1)",
+		},
+		{
+			"no wrapping - email field",
+			"select email as id, bool from demo1",
+			"CREATE VIEW `test_wrapping` AS SELECT * FROM (select email as id, bool from demo1)",
+		},
+		{
+			"no wrapping - datetime field",
+			"select datetime as id, bool from demo1",
+			"CREATE VIEW `test_wrapping` AS SELECT * FROM (select datetime as id, bool from demo1)",
+		},
+		{
+			"no wrapping - url field",
+			"select url as id, bool from demo1",
+			"CREATE VIEW `test_wrapping` AS SELECT * FROM (select url as id, bool from demo1)",
+		},
+		{
+			"wrapping - bool field",
+			"select bool as id, text as txt, url from demo1",
+			"CREATE VIEW `test_wrapping` AS SELECT * FROM (SELECT cast(id as text) id,txt,url FROM (select bool as id, text as txt, url from demo1))",
+		},
+		{
+			"wrapping - bool field (different order)",
+			"select text as txt, url, bool as id from demo1",
+			"CREATE VIEW `test_wrapping` AS SELECT * FROM (SELECT txt,url,cast(id as text) id FROM (select text as txt, url, bool as id from demo1))",
+		},
+		{
+			"wrapping - json field",
+			"select json as id, text, url from demo1",
+			"CREATE VIEW `test_wrapping` AS SELECT * FROM (SELECT cast(id as text) id,text,url FROM (select json as id, text, url from demo1))",
+		},
+		{
+			"wrapping - numeric id",
+			"select 1 as id",
+			"CREATE VIEW `test_wrapping` AS SELECT * FROM (SELECT cast(id as text) id FROM (select 1 as id))",
+		},
+		{
+			"wrapping - expresion",
+			"select ('test') as id",
+			"CREATE VIEW `test_wrapping` AS SELECT * FROM (SELECT cast(id as text) id FROM (select ('test') as id))",
+		},
+		{
+			"no wrapping - cast as text",
+			"select cast('test' as text) as id",
+			"CREATE VIEW `test_wrapping` AS SELECT * FROM (select cast('test' as text) as id)",
+		},
+	}
+
+	for _, s := range scenarios {
+		t.Run(s.name, func(t *testing.T) {
+			app, _ := tests.NewTestApp()
+			defer app.Cleanup()
+
+			collection := &models.Collection{
+				Name: viewName,
+				Type: models.CollectionTypeView,
+				Options: types.JsonMap{
+					"query": s.query,
+				},
+			}
+
+			err := app.Dao().SaveCollection(collection)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			var sql string
+
+			rowErr := app.Dao().DB().NewQuery("SELECT sql FROM sqlite_master WHERE type='view' AND name={:name}").
+				Bind(dbx.Params{"name": viewName}).
+				Row(&sql)
+			if rowErr != nil {
+				t.Fatalf("Failed to retrieve view sql: %v", rowErr)
+			}
+
+			if sql != s.expected {
+				t.Fatalf("Expected query \n%v, \ngot \n%v", s.expected, sql)
+			}
+		})
+	}
+}
+
 func TestImportCollections(t *testing.T) {
-	totalCollections := 10
+	totalCollections := 11
 
 	scenarios := []struct {
 		name                   string

daos/view.go

@@ -235,6 +235,8 @@ func defaultViewField(name string) *schema.SchemaField {
 	}
 }
 
+var castRegex = regexp.MustCompile(`(?i)^cast\s*\(.*\s+as\s+(\w+)\s*\)$`)
+
 func (dao *Dao) parseQueryToFields(selectQuery string) (map[string]*queryField, error) {
 	p := new(identifiersParser)
 	if err := p.parse(selectQuery); err != nil {
@@ -257,15 +259,8 @@ func (dao *Dao) parseQueryToFields(selectQuery string) (map[string]*queryField,
 	for _, col := range p.columns {
 		colLower := strings.ToLower(col.original)
 
-		// numeric expression cast
-		if strings.Contains(colLower, "(") &&
-			(strings.HasPrefix(colLower, "count(") ||
-				strings.HasPrefix(colLower, "total(") ||
-				strings.Contains(colLower, " as numeric") ||
-				strings.Contains(colLower, " as real") ||
-				strings.Contains(colLower, " as int") ||
-				strings.Contains(colLower, " as integer") ||
-				strings.Contains(colLower, " as decimal")) {
+		// numeric aggregations
+		if strings.HasPrefix(colLower, "count(") || strings.HasPrefix(colLower, "total(") {
 			result[col.alias] = &queryField{
 				field: &schema.SchemaField{
 					Name: col.alias,
@@ -275,6 +270,38 @@ func (dao *Dao) parseQueryToFields(selectQuery string) (map[string]*queryField,
 			continue
 		}
 
+		castMatch := castRegex.FindStringSubmatch(colLower)
+
+		// numeric casts
+		if len(castMatch) == 2 {
+			switch castMatch[1] {
+			case "real", "integer", "int", "decimal", "numeric":
+				result[col.alias] = &queryField{
+					field: &schema.SchemaField{
+						Name: col.alias,
+						Type: schema.FieldTypeNumber,
+					},
+				}
+				continue
+			case "text":
+				result[col.alias] = &queryField{
+					field: &schema.SchemaField{
+						Name: col.alias,
+						Type: schema.FieldTypeText,
+					},
+				}
+				continue
+			case "boolean", "bool":
+				result[col.alias] = &queryField{
+					field: &schema.SchemaField{
+						Name: col.alias,
+						Type: schema.FieldTypeBool,
+					},
+				}
+				continue
+			}
+		}
+
 		parts := strings.Split(col.original, ".")
 
 		var fieldName string