Skip to content

Commit

Permalink
Don't expose internal '::' separators to clients (#524)
Browse files Browse the repository at this point in the history
Double colons are our internal implementation detail - described
[here](https://github.com/QuesmaOrg/quesma/blob/399fd231bb97437687bc18b88a734939fe47357e/adr/4_double_colon_as_nested_structure_separator.md).
Users should not know about it, but use classic dots as separators.

depends on:
- #526
- #525
- #527
  • Loading branch information
pivovarit authored Jul 14, 2024
1 parent 4dd7dba commit dbc1d30
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 47 deletions.
3 changes: 2 additions & 1 deletion examples/kibana-sample-data/quesma/config/local-dev.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ indexes:
products.product_name: "text"
geoip.location: "geo_point"
category: "text"
manufacturer: "text"
kibana_sample_data_flights:
enabled: true
mappings:
Expand All @@ -36,7 +37,7 @@ indexes:
mappings:
ip: "ip"
clientip: "ip"
geo::coordinates: "point"
geo.coordinates: "point"
extension: "text"
url: "text"
aliases:
Expand Down
2 changes: 1 addition & 1 deletion http_requests/field_caps.http
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
POST http://localhost:8080/kibana_sample_data_flights/_field_caps
POST http://localhost:9200/kibana_sample_data_flights/_field_caps
Content-Type: application/json

{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ func doubleColons2dot(input string) string {
return strings.ReplaceAll(input, doubleColons, dot)
}

func dot2DoubleColons(input string) string {
return strings.ReplaceAll(input, dot, doubleColons)
}

func dot2SQLNative(input string) string {
return strings.ReplaceAll(input, dot, sqlNative)
}
Expand Down Expand Up @@ -259,8 +255,7 @@ func (p *Dot2DoubleColons) GetTableColumnFormatter(table string, cfg config.Ques
type Dot2DoubleColons2Dot struct{}

func (*Dot2DoubleColons2Dot) matches(table string) bool {
// this is enabled for e-commerce data, it makes dashboard work
return strings.HasPrefix(table, "kibana_sample_data_ecommerce")
return true
}

func (*Dot2DoubleColons2Dot) IngestTransformer() plugins.IngestTransformer {
Expand All @@ -282,9 +277,6 @@ func (p *Dot2DoubleColons2Dot) GetTableColumnFormatter(table string, cfg config.
}

func (p *Dot2DoubleColons2Dot) ApplyQueryTransformers(table string, cfg config.QuesmaConfiguration, transformers []plugins.QueryTransformer) []plugins.QueryTransformer {
if p.matches(table) {
transformers = append(transformers, &queryTransformer{translate: dot2DoubleColons})
}
return transformers
}

Expand All @@ -296,9 +288,6 @@ func (p *Dot2DoubleColons2Dot) ApplyResultTransformers(table string, cfg config.
}

func (p *Dot2DoubleColons2Dot) ApplyFieldCapsTransformers(table string, cfg config.QuesmaConfiguration, transformers []plugins.FieldCapsTransformer) []plugins.FieldCapsTransformer {
if p.matches(table) {
transformers = append(transformers, &fieldCapsTransformer{translate: doubleColons2dot})
}
return transformers
}

Expand Down
7 changes: 1 addition & 6 deletions quesma/plugins/registry/plugin_registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,7 @@ import (
var registeredPlugins []plugins.Plugin

func init() {
registeredPlugins = []plugins.Plugin{
// TODO below plugins are disabled due to some
// interferences with other components
&elastic_clickhouse_fields.Dot2DoubleColons2Dot{},
//&elastic_clickhouse_fields.Dot2DoubleUnderscores2Dot{},
&elastic_clickhouse_fields.Dot2DoubleColons{}}
registeredPlugins = []plugins.Plugin{&elastic_clickhouse_fields.Dot2DoubleColons2Dot{}}
}

func QueryTransformerFor(table string, cfg config.QuesmaConfiguration) plugins.QueryTransformer {
Expand Down
36 changes: 15 additions & 21 deletions quesma/queryparser/query_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -931,12 +931,9 @@ func (cw *ClickhouseQueryTranslator) parseExists(queryMap QueryMap) model.Simple
), "=", model.NewLiteral("0"))
case clickhouse.NotExists:
// TODO this is a workaround for the case when the field is a point
schemaInstance, exists := cw.SchemaRegistry.FindSchema(schema.TableName(cw.Table.Name))
if exists {
if value, ok := schemaInstance.Fields[schema.FieldName(fieldName)]; ok {
if value.Type.Equal(schema.TypePoint) {
return model.NewSimpleQuery(sql, true)
}
if schemaInstance, exists := cw.SchemaRegistry.FindSchema(schema.TableName(cw.Table.Name)); exists {
if value, ok := schemaInstance.ResolveFieldByInternalName(fieldName); ok && value.Type.Equal(schema.TypePoint) {
return model.NewSimpleQuery(sql, true)
}
}
attrs := cw.Table.GetAttributesList()
Expand Down Expand Up @@ -1345,33 +1342,30 @@ func createSortColumn(fieldName, ordering string) (model.OrderByExpr, error) {
// What prevents us from moving it to transformation pipeline now, is that
// we need to anotate this field somehow in the AST, to be able
// to distinguish it from other fields
func (cw *ClickhouseQueryTranslator) ResolveField(ctx context.Context, fieldName string) (field string) {
func (cw *ClickhouseQueryTranslator) ResolveField(ctx context.Context, fieldName string) string {
// Alias resolution should occur *after* the query is parsed, not during the parsing
if cw.SchemaRegistry == nil {
logger.Error().Msg("Schema registry is not set")
field = fieldName
return
return fieldName
}
schemaInstance, exists := cw.SchemaRegistry.FindSchema(schema.TableName(cw.Table.Name))
if !exists {
logger.Error().Msgf("Schema for table [%s] not found, this should never happen", cw.Table.Name)
field = fieldName
return
return fieldName
}
// TODO this should be handled eventually at schema level
fieldName = strings.TrimSuffix(fieldName, ".keyword")
fieldName = strings.TrimSuffix(fieldName, ".text")

if resolvedField, ok := schemaInstance.ResolveField(fieldName); ok {
field = resolvedField.InternalPropertyName.AsString()
return resolvedField.InternalPropertyName.AsString()
} else {
// fallback to original field name
logger.DebugWithCtx(ctx).Msgf("field '%s' referenced, but not found in schema", fieldName)
field = fieldName
}

if field != "*" && field != "_all" && field != "_doc" && field != "_id" && field != "_index" {
if _, ok := schemaInstance.Fields[schema.FieldName(field)]; !ok {
logger.DebugWithCtx(ctx).Msgf("field '%s' referenced, but not found in schema", fieldName)
if fieldName != "*" && fieldName != "_all" && fieldName != "_doc" && fieldName != "_id" && fieldName != "_index" {
logger.DebugWithCtx(ctx).Msgf("field '%s' referenced, but not found in schema, falling back to original name", fieldName)
}

return fieldName
}
return
}
func (cw *ClickhouseQueryTranslator) parseSizeExists(queryMap QueryMap) (size int, ok bool) {
sizeRaw, exists := queryMap["size"]
Expand Down
26 changes: 20 additions & 6 deletions quesma/schema/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package schema
import (
"quesma/logger"
"quesma/quesma/config"
"strings"
)

type (
Expand Down Expand Up @@ -84,7 +85,12 @@ func (s *schemaRegistry) populateSchemaFromStaticConfiguration(indexConfiguratio
if deprecatedConfigInUse(indexConfiguration) {
for fieldName, fieldType := range indexConfiguration.TypeMappings {
if resolvedType, valid := ParseType(fieldType); valid {
fields[FieldName(fieldName)] = Field{PropertyName: FieldName(fieldName), InternalPropertyName: FieldName(fieldName), Type: resolvedType}
if resolvedType.Equal(TypePoint) {
// TODO replace with notion of ephemeral types
fields[FieldName(fieldName)] = Field{PropertyName: FieldName(fieldName), InternalPropertyName: FieldName(strings.Replace(fieldName, ".", "::", -1)), Type: resolvedType}
} else {
fields[FieldName(fieldName)] = Field{PropertyName: FieldName(fieldName), InternalPropertyName: FieldName(fieldName), Type: resolvedType}
}
} else {
logger.Warn().Msgf("invalid configuration: type %s not supported (should have been spotted when validating configuration)", fieldType)
}
Expand All @@ -95,7 +101,12 @@ func (s *schemaRegistry) populateSchemaFromStaticConfiguration(indexConfiguratio
continue
}
if resolvedType, valid := ParseType(field.Type.AsString()); valid {
fields[FieldName(field.Name.AsString())] = Field{PropertyName: FieldName(field.Name.AsString()), InternalPropertyName: FieldName(field.Name.AsString()), Type: resolvedType}
// TODO replace with notion of ephemeral types
if resolvedType.Equal(TypePoint) {
fields[FieldName(field.Name.AsString())] = Field{PropertyName: FieldName(field.Name.AsString()), InternalPropertyName: FieldName(strings.Replace(field.Name.AsString(), ".", "::", -1)), Type: resolvedType}
} else {
fields[FieldName(field.Name.AsString())] = Field{PropertyName: FieldName(field.Name.AsString()), InternalPropertyName: FieldName(field.Name.AsString()), Type: resolvedType}
}
} else {
logger.Warn().Msgf("invalid configuration: type %s not supported (should have been spotted when validating configuration)", field.Type.AsString())
}
Expand Down Expand Up @@ -130,13 +141,16 @@ func (s *schemaRegistry) populateSchemaFromTableDefinition(definitions map[strin
logger.Debug().Msgf("loading schema for table %s", indexName)

for _, column := range tableDefinition.Columns {
if _, exists := fields[FieldName(column.Name)]; !exists {
if quesmaType, found2 := s.dataSourceTypeAdapter.Convert(column.Type); found2 {
fields[FieldName(column.Name)] = Field{PropertyName: FieldName(column.Name), InternalPropertyName: FieldName(column.Name), Type: quesmaType}
propertyName := FieldName(strings.Replace(column.Name, "::", ".", -1))
if existing, exists := fields[propertyName]; !exists {
if quesmaType, resolved := s.dataSourceTypeAdapter.Convert(column.Type); resolved {
fields[propertyName] = Field{PropertyName: propertyName, InternalPropertyName: FieldName(column.Name), Type: quesmaType}
} else {
logger.Debug().Msgf("type %s not supported, falling back to text", column.Type)
fields[FieldName(column.Name)] = Field{PropertyName: FieldName(column.Name), InternalPropertyName: FieldName(column.Name), Type: TypeText}
fields[propertyName] = Field{PropertyName: propertyName, InternalPropertyName: FieldName(column.Name), Type: TypeText}
}
} else {
fields[propertyName] = Field{PropertyName: propertyName, InternalPropertyName: FieldName(column.Name), Type: existing.Type}
}
}
}
Expand Down

0 comments on commit dbc1d30

Please sign in to comment.