Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement default mapper for SQL visibility #3875

Merged
merged 1 commit into from
Feb 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions common/resource/fx.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,15 @@ func TimeSourceProvider() clock.TimeSource {

func SearchAttributeMapperProviderProvider(
saMapper searchattribute.Mapper,
namespaceRegistry namespace.Registry,
searchAttributeProvider searchattribute.Provider,
persistenceConfig *config.Persistence,
) searchattribute.MapperProvider {
return searchattribute.NewMapperProvider(
saMapper,
namespaceRegistry,
searchAttributeProvider,
persistenceConfig.IsSQLVisibilityStore(),
)
}

Expand Down
69 changes: 65 additions & 4 deletions common/searchattribute/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,29 @@ type (

noopMapper struct{}

// This mapper is to be backwards compatible with versions before v1.20.
// Users using standard visibility might have registered custom search attributes.
// Those search attributes won't be searchable, as they weren't before version v1.20.
// Thus, this mapper will allow those search attributes to be used without being alised.
backCompMapper_v1_20 struct {
mapper Mapper
emptyStringNameTypeMap NameTypeMap
}

MapperProvider interface {
GetMapper(nsName namespace.Name) (Mapper, error)
}

mapperProviderImpl struct {
customMapper Mapper
customMapper Mapper
namespaceRegistry namespace.Registry
searchAttributesProvider Provider
enableMapperFromNamespace bool
}
)

var _ Mapper = (*noopMapper)(nil)
var _ Mapper = (*backCompMapper_v1_20)(nil)
var _ Mapper = (*namespace.CustomSearchAttributesMapper)(nil)
var _ MapperProvider = (*mapperProviderImpl)(nil)

Expand All @@ -64,16 +77,64 @@ func (m *noopMapper) GetFieldName(alias string, _ string) (string, error) {
return alias, nil
}

func (m *backCompMapper_v1_20) GetAlias(fieldName string, namespaceName string) (string, error) {
alias, firstErr := m.mapper.GetAlias(fieldName, namespaceName)
if firstErr != nil {
_, err := m.emptyStringNameTypeMap.getType(fieldName, customCategory)
if err != nil {
return "", firstErr
}
// this is custom search attribute registered in pre-v1.20
return fieldName, nil
}
return alias, nil
}

func (m *backCompMapper_v1_20) GetFieldName(alias string, namespaceName string) (string, error) {
fieldName, firstErr := m.mapper.GetFieldName(alias, namespaceName)
if firstErr != nil {
_, err := m.emptyStringNameTypeMap.getType(alias, customCategory)
if err != nil {
return "", firstErr
}
// this is custom search attribute registered in pre-v1.20
return alias, nil
}
return fieldName, nil
}

func NewMapperProvider(
customMapper Mapper,
namespaceRegistry namespace.Registry,
searchAttributesProvider Provider,
enableMapperFromNamespace bool,
) MapperProvider {
return &mapperProviderImpl{
customMapper: customMapper,
customMapper: customMapper,
namespaceRegistry: namespaceRegistry,
searchAttributesProvider: searchAttributesProvider,
enableMapperFromNamespace: enableMapperFromNamespace,
}
}

func (m *mapperProviderImpl) GetMapper(_ namespace.Name) (Mapper, error) {
return m.customMapper, nil
func (m *mapperProviderImpl) GetMapper(nsName namespace.Name) (Mapper, error) {
if m.customMapper != nil {
return m.customMapper, nil
}
if !m.enableMapperFromNamespace {
return &noopMapper{}, nil
}
ns, err := m.namespaceRegistry.GetNamespace(nsName)
if err != nil {
return nil, err
}
saMapper := ns.CustomSearchAttributesMapper()
// if there's an error, it returns an empty object, which is expected here
emptyStringNameTypeMap, _ := m.searchAttributesProvider.GetSearchAttributes("", false)
return &backCompMapper_v1_20{
mapper: &saMapper,
emptyStringNameTypeMap: emptyStringNameTypeMap,
}, nil
}

// AliasFields returns SearchAttributes struct where each search attribute name is replaced with alias.
Expand Down
9 changes: 8 additions & 1 deletion common/searchattribute/test_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ var (
"CustomDatetimeField": enumspb.INDEXED_VALUE_TYPE_DATETIME,
"CustomDoubleField": enumspb.INDEXED_VALUE_TYPE_DOUBLE,
"CustomBoolField": enumspb.INDEXED_VALUE_TYPE_BOOL,

"Int01": enumspb.INDEXED_VALUE_TYPE_INT,
"Text01": enumspb.INDEXED_VALUE_TYPE_TEXT,
"Keyword01": enumspb.INDEXED_VALUE_TYPE_KEYWORD,
"Datetime01": enumspb.INDEXED_VALUE_TYPE_DATETIME,
"Double01": enumspb.INDEXED_VALUE_TYPE_DOUBLE,
"Bool01": enumspb.INDEXED_VALUE_TYPE_BOOL,
},
}
)
Expand Down Expand Up @@ -102,5 +109,5 @@ func (t *TestMapper) GetFieldName(alias string, namespace string) (string, error
}

func NewTestMapperProvider(customMapper Mapper) MapperProvider {
return NewMapperProvider(customMapper)
return NewMapperProvider(customMapper, nil, NewTestProvider(), false)
}
7 changes: 7 additions & 0 deletions common/searchattribute/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ func (v *Validator) Validate(searchAttributes *commonpb.SearchAttributes, namesp
)
}

// TODO (rodrigozhou): this is to be backwards compatible with custom search attributes
// registered before v1.20. Ignoring error as this key might not exist.
emptyStringSaTypeMap, _ := v.searchAttributesProvider.GetSearchAttributes("", false)

for saFieldName, saPayload := range searchAttributes.GetIndexedFields() {
// user search attribute cannot be a system search attribute
if _, err = saTypeMap.getType(saFieldName, systemCategory); err == nil {
Expand All @@ -101,6 +105,9 @@ func (v *Validator) Validate(searchAttributes *commonpb.SearchAttributes, namesp
}

saType, err := saTypeMap.getType(saFieldName, customCategory|predefinedCategory)
if err != nil {
saType, err = emptyStringSaTypeMap.getType(saFieldName, customCategory)
}
if err != nil {
if errors.Is(err, ErrInvalidName) {
return v.validationError(
Expand Down
110 changes: 110 additions & 0 deletions develop/buildkite/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,33 @@ services:
aliases:
- integration-test

integration-test-mysql8:
build:
context: ../..
dockerfile: ./develop/buildkite/Dockerfile
environment:
- "MYSQL_SEEDS=mysql"
- "ES_SEEDS=elasticsearch"
- "ES_VERSION=v7"
- "PERSISTENCE_TYPE=sql"
- "PERSISTENCE_DRIVER=mysql8"
- "TEST_TAG=esintegration"
- "TEMPORAL_VERSION_CHECK_DISABLED=1"
- BUILDKITE_AGENT_ACCESS_TOKEN
- BUILDKITE_JOB_ID
- BUILDKITE_BUILD_ID
- BUILDKITE_BUILD_NUMBER
depends_on:
- mysql
- elasticsearch
volumes:
- ../..:/temporal
- /usr/bin/buildkite-agent:/usr/bin/buildkite-agent
networks:
services-network:
aliases:
- integration-test

integration-test-postgresql:
build:
context: ../..
Expand Down Expand Up @@ -164,6 +191,33 @@ services:
aliases:
- integration-test

integration-test-postgresql12:
build:
context: ../..
dockerfile: ./develop/buildkite/Dockerfile
environment:
- "POSTGRES_SEEDS=postgresql"
- "ES_SEEDS=elasticsearch"
- "ES_VERSION=v7"
- "PERSISTENCE_TYPE=sql"
- "PERSISTENCE_DRIVER=postgres12"
- "TEST_TAG=esintegration"
- "TEMPORAL_VERSION_CHECK_DISABLED=1"
- BUILDKITE_AGENT_ACCESS_TOKEN
- BUILDKITE_JOB_ID
- BUILDKITE_BUILD_ID
- BUILDKITE_BUILD_NUMBER
depends_on:
- postgresql
- elasticsearch
volumes:
- ../..:/temporal
- /usr/bin/buildkite-agent:/usr/bin/buildkite-agent
networks:
services-network:
aliases:
- integration-test

integration-test-sqlite:
build:
context: ../..
Expand Down Expand Up @@ -240,6 +294,34 @@ services:
aliases:
- integration-test-xdc

integration-test-xdc-mysql8:
build:
context: ../..
dockerfile: ./develop/buildkite/Dockerfile
environment:
- "MYSQL_SEEDS=mysql"
- "ES_SEEDS=elasticsearch"
- "ES_VERSION=v7"
- "PERSISTENCE_TYPE=sql"
- "PERSISTENCE_DRIVER=mysql8"
- "TEST_TAG=esintegration"
- "TEST_RUN_COUNT=10"
- "TEMPORAL_VERSION_CHECK_DISABLED=1"
- BUILDKITE_AGENT_ACCESS_TOKEN
- BUILDKITE_JOB_ID
- BUILDKITE_BUILD_ID
- BUILDKITE_BUILD_NUMBER
depends_on:
- mysql
- elasticsearch
volumes:
- ../..:/temporal
- /usr/bin/buildkite-agent:/usr/bin/buildkite-agent
networks:
services-network:
aliases:
- integration-test-xdc

integration-test-xdc-postgresql:
build:
context: ../..
Expand Down Expand Up @@ -268,6 +350,34 @@ services:
aliases:
- integration-test-xdc

integration-test-xdc-postgresql12:
build:
context: ../..
dockerfile: ./develop/buildkite/Dockerfile
environment:
- "POSTGRES_SEEDS=postgresql"
- "ES_SEEDS=elasticsearch"
- "ES_VERSION=v7"
- "PERSISTENCE_TYPE=sql"
- "PERSISTENCE_DRIVER=postgres12"
- "TEST_TAG=esintegration"
- "TEST_RUN_COUNT=10"
- "TEMPORAL_VERSION_CHECK_DISABLED=1"
- BUILDKITE_AGENT_ACCESS_TOKEN
- BUILDKITE_JOB_ID
- BUILDKITE_BUILD_ID
- BUILDKITE_BUILD_NUMBER
depends_on:
- postgresql
- elasticsearch
volumes:
- ../..:/temporal
- /usr/bin/buildkite-agent:/usr/bin/buildkite-agent
networks:
services-network:
aliases:
- integration-test-xdc

coverage-report:
build:
context: ../..
Expand Down
90 changes: 90 additions & 0 deletions develop/buildkite/pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,51 @@ steps:
run: integration-test-xdc-mysql
config: ./develop/buildkite/docker-compose.yml

- label: ":golang: functional test with mysql 8"
agents:
queue: "default"
docker: "*"
command: "make functional-test-coverage"
artifact_paths:
- ".coverage/*.out"
retry:
automatic:
limit: 1
plugins:
- docker-compose#v3.8.0:
run: integration-test-mysql8
config: ./develop/buildkite/docker-compose.yml

- label: ":golang: functional xdc test with mysql 8"
agents:
queue: "default"
docker: "*"
command: "make functional-test-xdc-coverage"
artifact_paths:
- ".coverage/*.out"
retry:
automatic:
limit: 1
plugins:
- docker-compose#v3.8.0:
run: integration-test-xdc-mysql8
config: ./develop/buildkite/docker-compose.yml

- label: ":golang: functional ndc test with mysql 8"
agents:
queue: "default"
docker: "*"
command: "make functional-test-ndc-coverage"
artifact_paths:
- ".coverage/*.out"
retry:
automatic:
limit: 1
plugins:
- docker-compose#v3.8.0:
run: integration-test-xdc-mysql8
config: ./develop/buildkite/docker-compose.yml

- label: ":golang: functional test with postgresql"
agents:
queue: "default"
Expand Down Expand Up @@ -185,6 +230,51 @@ steps:
run: integration-test-xdc-postgresql
config: ./develop/buildkite/docker-compose.yml

- label: ":golang: functional test with postgresql 12"
agents:
queue: "default"
docker: "*"
command: "make functional-test-coverage"
artifact_paths:
- ".coverage/*.out"
retry:
automatic:
limit: 1
plugins:
- docker-compose#v3.8.0:
run: integration-test-postgresql12
config: ./develop/buildkite/docker-compose.yml

- label: ":golang: functional xdc test with postgresql 12"
agents:
queue: "default"
docker: "*"
command: "make functional-test-xdc-coverage"
artifact_paths:
- ".coverage/*.out"
retry:
automatic:
limit: 1
plugins:
- docker-compose#v3.8.0:
run: integration-test-xdc-postgresql12
config: ./develop/buildkite/docker-compose.yml

- label: ":golang: functional ndc test with postgresql 12"
agents:
queue: "default"
docker: "*"
command: "make functional-test-ndc-coverage"
artifact_paths:
- ".coverage/*.out"
retry:
automatic:
limit: 1
plugins:
- docker-compose#v3.8.0:
run: integration-test-xdc-postgresql12
config: ./develop/buildkite/docker-compose.yml

- label: ":golang: functional test with sqlite"
agents:
queue: "default"
Expand Down
Loading