store api labels as json, filter using sql #1219
Conversation
|
JFYI, the failing tests are because of a couple of CEL conversion failures:
|
|
For posterity, if we return to this we may also want to look at indexing the JSON. Some references: |
| @@ -35,8 +36,8 @@ type Api struct { | |||
| Availability string // Availability of the API. | |||
| RecommendedVersion string // Recommended API version. | |||
| RecommendedDeployment string // Recommended API deployment. | |||
| Labels []byte // Serialized labels. | |||
| Annotations []byte // Serialized annotations. | |||
| Labels datatypes.JSON | |||
There was a problem hiding this comment.
Do you know what this is using at the database level? I'm guessing/hoping json or jsonb for Postgres, but have no idea what it's doing for SQLite (bytes?).
There was a problem hiding this comment.
SQLite natively supports JSON and GORM creates this as a JSON type column.
| return Filter{}, status.Error(codes.InvalidArgument, err.Error()) | ||
| } | ||
| // TODO: quick hack to adjust field name | ||
| sqlCondition = strings.ReplaceAll(sqlCondition, "name", "key") |
There was a problem hiding this comment.
Wow, I'm amazed that cel2sql.Convert() works with so little modification. I was thinking that we would have to own this parsing/conversion. I expect it will at least need to be a grey box to us (if we use it, we'll at least be stepping inside it to understand/debug it)
There was a problem hiding this comment.
Eh... well, it "works" for simple cases. It's not complete and it's created specifically for Big Query, so most likely we'll have to fork... and even then, as I mentioned, I think we'll have to have a fallback path for some CEL.
There was a problem hiding this comment.
(I just read the comment about the failing tests, that's in line with this)
There was a problem hiding this comment.
Impressive - do you see an easy way to structure this to be optional? That looks difficult with the gorm annotations on our table structures, but it would be nice to have this behind a feature flag. If we can't do that, we might want to start an experimental branch to take this further.
Not for the data, but I'm not sure that's necessary... it should be easy to move from binary blob to JSON and change nothing but the encoding and decoding. As for switching between SQL/CEL and CEL-only? Yes. |
This is a quick proof of concept for storing labels as JSON and allowing our CEL filters to use SQL. An actual implementation would handle more cases and include a fallback process for filters that can't be properly converted.