Skip to content

Commit c257d84

Browse files
rodrigozhourodrigozhou
authored
Implement default mapper for SQL visibility (#3875)
1 parent 29afc2f commit c257d84

File tree

8 files changed

+329
-14
lines changed

8 files changed

+329
-14
lines changed

‎common/resource/fx.go

+6
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,15 @@ func TimeSourceProvider() clock.TimeSource {
165165

166166
func SearchAttributeMapperProviderProvider(
167167
saMapper searchattribute.Mapper,
168+
namespaceRegistry namespace.Registry,
169+
searchAttributeProvider searchattribute.Provider,
170+
persistenceConfig *config.Persistence,
168171
) searchattribute.MapperProvider {
169172
return searchattribute.NewMapperProvider(
170173
saMapper,
174+
namespaceRegistry,
175+
searchAttributeProvider,
176+
persistenceConfig.IsSQLVisibilityStore(),
171177
)
172178
}
173179

‎common/searchattribute/mapper.go

+65-4
Original file line numberDiff line numberDiff line change
@@ -43,16 +43,29 @@ type (
4343

4444
noopMapper struct{}
4545

46+
// This mapper is to be backwards compatible with versions before v1.20.
47+
// Users using standard visibility might have registered custom search attributes.
48+
// Those search attributes won't be searchable, as they weren't before version v1.20.
49+
// Thus, this mapper will allow those search attributes to be used without being alised.
50+
backCompMapper_v1_20 struct {
51+
mapper Mapper
52+
emptyStringNameTypeMap NameTypeMap
53+
}
54+
4655
MapperProvider interface {
4756
GetMapper(nsName namespace.Name) (Mapper, error)
4857
}
4958

5059
mapperProviderImpl struct {
51-
customMapper Mapper
60+
customMapper Mapper
61+
namespaceRegistry namespace.Registry
62+
searchAttributesProvider Provider
63+
enableMapperFromNamespace bool
5264
}
5365
)
5466

5567
var _ Mapper = (*noopMapper)(nil)
68+
var _ Mapper = (*backCompMapper_v1_20)(nil)
5669
var _ Mapper = (*namespace.CustomSearchAttributesMapper)(nil)
5770
var _ MapperProvider = (*mapperProviderImpl)(nil)
5871

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

80+
func (m *backCompMapper_v1_20) GetAlias(fieldName string, namespaceName string) (string, error) {
81+
alias, firstErr := m.mapper.GetAlias(fieldName, namespaceName)
82+
if firstErr != nil {
83+
_, err := m.emptyStringNameTypeMap.getType(fieldName, customCategory)
84+
if err != nil {
85+
return "", firstErr
86+
}
87+
// this is custom search attribute registered in pre-v1.20
88+
return fieldName, nil
89+
}
90+
return alias, nil
91+
}
92+
93+
func (m *backCompMapper_v1_20) GetFieldName(alias string, namespaceName string) (string, error) {
94+
fieldName, firstErr := m.mapper.GetFieldName(alias, namespaceName)
95+
if firstErr != nil {
96+
_, err := m.emptyStringNameTypeMap.getType(alias, customCategory)
97+
if err != nil {
98+
return "", firstErr
99+
}
100+
// this is custom search attribute registered in pre-v1.20
101+
return alias, nil
102+
}
103+
return fieldName, nil
104+
}
105+
67106
func NewMapperProvider(
68107
customMapper Mapper,
108+
namespaceRegistry namespace.Registry,
109+
searchAttributesProvider Provider,
110+
enableMapperFromNamespace bool,
69111
) MapperProvider {
70112
return &mapperProviderImpl{
71-
customMapper: customMapper,
113+
customMapper: customMapper,
114+
namespaceRegistry: namespaceRegistry,
115+
searchAttributesProvider: searchAttributesProvider,
116+
enableMapperFromNamespace: enableMapperFromNamespace,
72117
}
73118
}
74119

75-
func (m *mapperProviderImpl) GetMapper(_ namespace.Name) (Mapper, error) {
76-
return m.customMapper, nil
120+
func (m *mapperProviderImpl) GetMapper(nsName namespace.Name) (Mapper, error) {
121+
if m.customMapper != nil {
122+
return m.customMapper, nil
123+
}
124+
if !m.enableMapperFromNamespace {
125+
return &noopMapper{}, nil
126+
}
127+
ns, err := m.namespaceRegistry.GetNamespace(nsName)
128+
if err != nil {
129+
return nil, err
130+
}
131+
saMapper := ns.CustomSearchAttributesMapper()
132+
// if there's an error, it returns an empty object, which is expected here
133+
emptyStringNameTypeMap, _ := m.searchAttributesProvider.GetSearchAttributes("", false)
134+
return &backCompMapper_v1_20{
135+
mapper: &saMapper,
136+
emptyStringNameTypeMap: emptyStringNameTypeMap,
137+
}, nil
77138
}
78139

79140
// AliasFields returns SearchAttributes struct where each search attribute name is replaced with alias.

‎common/searchattribute/test_provider.go

+8-1
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,13 @@ var (
5151
"CustomDatetimeField": enumspb.INDEXED_VALUE_TYPE_DATETIME,
5252
"CustomDoubleField": enumspb.INDEXED_VALUE_TYPE_DOUBLE,
5353
"CustomBoolField": enumspb.INDEXED_VALUE_TYPE_BOOL,
54+
55+
"Int01": enumspb.INDEXED_VALUE_TYPE_INT,
56+
"Text01": enumspb.INDEXED_VALUE_TYPE_TEXT,
57+
"Keyword01": enumspb.INDEXED_VALUE_TYPE_KEYWORD,
58+
"Datetime01": enumspb.INDEXED_VALUE_TYPE_DATETIME,
59+
"Double01": enumspb.INDEXED_VALUE_TYPE_DOUBLE,
60+
"Bool01": enumspb.INDEXED_VALUE_TYPE_BOOL,
5461
},
5562
}
5663
)
@@ -102,5 +109,5 @@ func (t *TestMapper) GetFieldName(alias string, namespace string) (string, error
102109
}
103110

104111
func NewTestMapperProvider(customMapper Mapper) MapperProvider {
105-
return NewMapperProvider(customMapper)
112+
return NewMapperProvider(customMapper, nil, NewTestProvider(), false)
106113
}

‎common/searchattribute/validator.go

+7
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,10 @@ func (v *Validator) Validate(searchAttributes *commonpb.SearchAttributes, namesp
9292
)
9393
}
9494

95+
// TODO (rodrigozhou): this is to be backwards compatible with custom search attributes
96+
// registered before v1.20. Ignoring error as this key might not exist.
97+
emptyStringSaTypeMap, _ := v.searchAttributesProvider.GetSearchAttributes("", false)
98+
9599
for saFieldName, saPayload := range searchAttributes.GetIndexedFields() {
96100
// user search attribute cannot be a system search attribute
97101
if _, err = saTypeMap.getType(saFieldName, systemCategory); err == nil {
@@ -101,6 +105,9 @@ func (v *Validator) Validate(searchAttributes *commonpb.SearchAttributes, namesp
101105
}
102106

103107
saType, err := saTypeMap.getType(saFieldName, customCategory|predefinedCategory)
108+
if err != nil {
109+
saType, err = emptyStringSaTypeMap.getType(saFieldName, customCategory)
110+
}
104111
if err != nil {
105112
if errors.Is(err, ErrInvalidName) {
106113
return v.validationError(

‎develop/buildkite/docker-compose.yml

+110
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,33 @@ services:
137137
aliases:
138138
- integration-test
139139

140+
integration-test-mysql8:
141+
build:
142+
context: ../..
143+
dockerfile: ./develop/buildkite/Dockerfile
144+
environment:
145+
- "MYSQL_SEEDS=mysql"
146+
- "ES_SEEDS=elasticsearch"
147+
- "ES_VERSION=v7"
148+
- "PERSISTENCE_TYPE=sql"
149+
- "PERSISTENCE_DRIVER=mysql8"
150+
- "TEST_TAG=esintegration"
151+
- "TEMPORAL_VERSION_CHECK_DISABLED=1"
152+
- BUILDKITE_AGENT_ACCESS_TOKEN
153+
- BUILDKITE_JOB_ID
154+
- BUILDKITE_BUILD_ID
155+
- BUILDKITE_BUILD_NUMBER
156+
depends_on:
157+
- mysql
158+
- elasticsearch
159+
volumes:
160+
- ../..:/temporal
161+
- /usr/bin/buildkite-agent:/usr/bin/buildkite-agent
162+
networks:
163+
services-network:
164+
aliases:
165+
- integration-test
166+
140167
integration-test-postgresql:
141168
build:
142169
context: ../..
@@ -164,6 +191,33 @@ services:
164191
aliases:
165192
- integration-test
166193

194+
integration-test-postgresql12:
195+
build:
196+
context: ../..
197+
dockerfile: ./develop/buildkite/Dockerfile
198+
environment:
199+
- "POSTGRES_SEEDS=postgresql"
200+
- "ES_SEEDS=elasticsearch"
201+
- "ES_VERSION=v7"
202+
- "PERSISTENCE_TYPE=sql"
203+
- "PERSISTENCE_DRIVER=postgres12"
204+
- "TEST_TAG=esintegration"
205+
- "TEMPORAL_VERSION_CHECK_DISABLED=1"
206+
- BUILDKITE_AGENT_ACCESS_TOKEN
207+
- BUILDKITE_JOB_ID
208+
- BUILDKITE_BUILD_ID
209+
- BUILDKITE_BUILD_NUMBER
210+
depends_on:
211+
- postgresql
212+
- elasticsearch
213+
volumes:
214+
- ../..:/temporal
215+
- /usr/bin/buildkite-agent:/usr/bin/buildkite-agent
216+
networks:
217+
services-network:
218+
aliases:
219+
- integration-test
220+
167221
integration-test-sqlite:
168222
build:
169223
context: ../..
@@ -240,6 +294,34 @@ services:
240294
aliases:
241295
- integration-test-xdc
242296

297+
integration-test-xdc-mysql8:
298+
build:
299+
context: ../..
300+
dockerfile: ./develop/buildkite/Dockerfile
301+
environment:
302+
- "MYSQL_SEEDS=mysql"
303+
- "ES_SEEDS=elasticsearch"
304+
- "ES_VERSION=v7"
305+
- "PERSISTENCE_TYPE=sql"
306+
- "PERSISTENCE_DRIVER=mysql8"
307+
- "TEST_TAG=esintegration"
308+
- "TEST_RUN_COUNT=10"
309+
- "TEMPORAL_VERSION_CHECK_DISABLED=1"
310+
- BUILDKITE_AGENT_ACCESS_TOKEN
311+
- BUILDKITE_JOB_ID
312+
- BUILDKITE_BUILD_ID
313+
- BUILDKITE_BUILD_NUMBER
314+
depends_on:
315+
- mysql
316+
- elasticsearch
317+
volumes:
318+
- ../..:/temporal
319+
- /usr/bin/buildkite-agent:/usr/bin/buildkite-agent
320+
networks:
321+
services-network:
322+
aliases:
323+
- integration-test-xdc
324+
243325
integration-test-xdc-postgresql:
244326
build:
245327
context: ../..
@@ -268,6 +350,34 @@ services:
268350
aliases:
269351
- integration-test-xdc
270352

353+
integration-test-xdc-postgresql12:
354+
build:
355+
context: ../..
356+
dockerfile: ./develop/buildkite/Dockerfile
357+
environment:
358+
- "POSTGRES_SEEDS=postgresql"
359+
- "ES_SEEDS=elasticsearch"
360+
- "ES_VERSION=v7"
361+
- "PERSISTENCE_TYPE=sql"
362+
- "PERSISTENCE_DRIVER=postgres12"
363+
- "TEST_TAG=esintegration"
364+
- "TEST_RUN_COUNT=10"
365+
- "TEMPORAL_VERSION_CHECK_DISABLED=1"
366+
- BUILDKITE_AGENT_ACCESS_TOKEN
367+
- BUILDKITE_JOB_ID
368+
- BUILDKITE_BUILD_ID
369+
- BUILDKITE_BUILD_NUMBER
370+
depends_on:
371+
- postgresql
372+
- elasticsearch
373+
volumes:
374+
- ../..:/temporal
375+
- /usr/bin/buildkite-agent:/usr/bin/buildkite-agent
376+
networks:
377+
services-network:
378+
aliases:
379+
- integration-test-xdc
380+
271381
coverage-report:
272382
build:
273383
context: ../..

‎develop/buildkite/pipeline.yml

+90
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,51 @@ steps:
140140
run: integration-test-xdc-mysql
141141
config: ./develop/buildkite/docker-compose.yml
142142

143+
- label: ":golang: functional test with mysql 8"
144+
agents:
145+
queue: "default"
146+
docker: "*"
147+
command: "make functional-test-coverage"
148+
artifact_paths:
149+
- ".coverage/*.out"
150+
retry:
151+
automatic:
152+
limit: 1
153+
plugins:
154+
- docker-compose#v3.8.0:
155+
run: integration-test-mysql8
156+
config: ./develop/buildkite/docker-compose.yml
157+
158+
- label: ":golang: functional xdc test with mysql 8"
159+
agents:
160+
queue: "default"
161+
docker: "*"
162+
command: "make functional-test-xdc-coverage"
163+
artifact_paths:
164+
- ".coverage/*.out"
165+
retry:
166+
automatic:
167+
limit: 1
168+
plugins:
169+
- docker-compose#v3.8.0:
170+
run: integration-test-xdc-mysql8
171+
config: ./develop/buildkite/docker-compose.yml
172+
173+
- label: ":golang: functional ndc test with mysql 8"
174+
agents:
175+
queue: "default"
176+
docker: "*"
177+
command: "make functional-test-ndc-coverage"
178+
artifact_paths:
179+
- ".coverage/*.out"
180+
retry:
181+
automatic:
182+
limit: 1
183+
plugins:
184+
- docker-compose#v3.8.0:
185+
run: integration-test-xdc-mysql8
186+
config: ./develop/buildkite/docker-compose.yml
187+
143188
- label: ":golang: functional test with postgresql"
144189
agents:
145190
queue: "default"
@@ -185,6 +230,51 @@ steps:
185230
run: integration-test-xdc-postgresql
186231
config: ./develop/buildkite/docker-compose.yml
187232

233+
- label: ":golang: functional test with postgresql 12"
234+
agents:
235+
queue: "default"
236+
docker: "*"
237+
command: "make functional-test-coverage"
238+
artifact_paths:
239+
- ".coverage/*.out"
240+
retry:
241+
automatic:
242+
limit: 1
243+
plugins:
244+
- docker-compose#v3.8.0:
245+
run: integration-test-postgresql12
246+
config: ./develop/buildkite/docker-compose.yml
247+
248+
- label: ":golang: functional xdc test with postgresql 12"
249+
agents:
250+
queue: "default"
251+
docker: "*"
252+
command: "make functional-test-xdc-coverage"
253+
artifact_paths:
254+
- ".coverage/*.out"
255+
retry:
256+
automatic:
257+
limit: 1
258+
plugins:
259+
- docker-compose#v3.8.0:
260+
run: integration-test-xdc-postgresql12
261+
config: ./develop/buildkite/docker-compose.yml
262+
263+
- label: ":golang: functional ndc test with postgresql 12"
264+
agents:
265+
queue: "default"
266+
docker: "*"
267+
command: "make functional-test-ndc-coverage"
268+
artifact_paths:
269+
- ".coverage/*.out"
270+
retry:
271+
automatic:
272+
limit: 1
273+
plugins:
274+
- docker-compose#v3.8.0:
275+
run: integration-test-xdc-postgresql12
276+
config: ./develop/buildkite/docker-compose.yml
277+
188278
- label: ":golang: functional test with sqlite"
189279
agents:
190280
queue: "default"

0 commit comments

Comments
 (0)