Skip to content

CHASM Visibility ListWorkflowExecutions ignore unregistered CHASM search attributes #8773

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.

Sign up for GitHub

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

Merged
awln-temporal merged 4 commits into from
Dec 12, 2025
Merged

CHASM Visibility ListWorkflowExecutions ignore unregistered CHASM search attributes #8773

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f85db85
CHASM Visibility ListWorkflowExecutions ignore unregistered CHASM sea…
awln-temporal Dec 8, 2025
33a54b1
update string case handler
awln-temporal Dec 8, 2025
b720ddb
Merge branch 'main' into chasm-vis-read-ignore-unregistered
awln-temporal Dec 11, 2025
e242bd6
Merge branch 'main' into chasm-vis-read-ignore-unregistered
awln-temporal Dec 11, 2025
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
11 changes: 9 additions & 2 deletions common/persistence/visibility/store/sql/visibility_store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"go.temporal.io/server/common/persistence/visibility/store/query"
"go.temporal.io/server/common/resolver"
"go.temporal.io/server/common/searchattribute"
"go.temporal.io/server/common/searchattribute/sadefs"
)

type (
Expand Down Expand Up @@ -859,19 +860,25 @@ func (s *VisibilityStore) encodeRowSearchAttributes(
}

combinedTypeMap := store.CombineTypeMaps(saTypeMap, chasmMapper)
registeredSearchAttributes := sqlplugin.VisibilitySearchAttributes{}

// Fix SQLite keyword list handling (convert string to []string for keyword lists)
for name, value := range rowSearchAttributes {
tp, err := combinedTypeMap.GetType(name)
if err != nil {
// Silently ignore ErrInvalidName for unregistered chasm search attributes
if sadefs.IsChasmSearchAttribute(name) && errors.Is(err, searchattribute.ErrInvalidName) {
Copy link
Contributor

@rodrigozhou rodrigozhou Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The combinedTypeMap already contains the CHASM search attributes. Do you need to check IsChasmSearchAttribute here?

Copy link
Contributor Author

@awln-temporal awln-temporal Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, since the chasm search search attributes are not registered when calling ListWorkflowExecutions.

Copy link
Contributor

@rodrigozhou rodrigozhou Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, in this case, isn't it better to check before calling combinedTypeMap.GetType?

Copy link
Contributor Author

@awln-temporal awln-temporal Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, since the chasm search attribute could be registered, in which case we do want to try returning it. visibility store implementations are unified for ListWorkflowExecutions and ListChasmExecutions as of now.

continue
}
return nil, err
}
registeredSearchAttributes[name] = value
if tp == enumspb.INDEXED_VALUE_TYPE_KEYWORD_LIST {
switch v := value.(type) {
case []string:
// no-op
case string:
rowSearchAttributes[name] = []string{v}
registeredSearchAttributes[name] = []string{v}
default:
return nil, serviceerror.NewInternal(
fmt.Sprintf("Unexpected data type for keyword list: %T (expected list of strings)", v),
Expand All @@ -881,7 +888,7 @@ func (s *VisibilityStore) encodeRowSearchAttributes(
}

// Encode all search attributes together
encodedSAs, err := searchattribute.Encode(rowSearchAttributes, &combinedTypeMap)
encodedSAs, err := searchattribute.Encode(registeredSearchAttributes, &combinedTypeMap)
if err != nil {
return nil, err
}
Expand Down
64 changes: 64 additions & 0 deletions tests/chasm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (

"github.com/stretchr/testify/suite"
"go.temporal.io/api/serviceerror"
workflowpb "go.temporal.io/api/workflow/v1"
"go.temporal.io/api/workflowservice/v1"
"go.temporal.io/server/chasm"
"go.temporal.io/server/chasm/lib/tests"
"go.temporal.io/server/chasm/lib/tests/gen/testspb/v1"
Expand Down Expand Up @@ -433,4 +435,66 @@ func (s *ChasmTestSuite) TestCountExecutions_GroupBy() {
s.Contains(err.Error(), "GROUP BY")
}

func (s *ChasmTestSuite) TestListWorkflowExecutions() {
tv := testvars.New(s.T())
ctx, cancel := context.WithTimeout(s.chasmContext, chasmTestTimeout)
defer cancel()

storeID := tv.Any().String()
createResp, err := tests.NewPayloadStoreHandler(
ctx,
tests.NewPayloadStoreRequest{
NamespaceID: s.NamespaceID(),
StoreID: storeID,
},
)
s.NoError(err)

_, err = tests.AddPayloadHandler(
ctx,
tests.AddPayloadRequest{
NamespaceID: s.NamespaceID(),
StoreID: storeID,
PayloadKey: "test-key",
Payload: payload.EncodeString("test-value"),
},
)
s.NoError(err)

visQuery := sadefs.QueryWithAnyNamespaceDivision(
fmt.Sprintf("WorkflowId = '%s'", storeID),
)

var execInfo *workflowpb.WorkflowExecutionInfo
s.Eventually(
func() bool {
listResp, err := s.FrontendClient().ListWorkflowExecutions(testcore.NewContext(), &workflowservice.ListWorkflowExecutionsRequest{
Namespace: s.Namespace().String(),
PageSize: 10,
Query: visQuery,
})
s.NoError(err)
if len(listResp.Executions) != 1 {
return false
}
execInfo = listResp.Executions[0]
return true
},
testcore.WaitForESToSettle,
100*time.Millisecond,
)

s.Equal(storeID, execInfo.Execution.WorkflowId)
s.Equal(createResp.RunID, execInfo.Execution.RunId)

s.NotNil(execInfo.SearchAttributes)
_, hasScheduledByID := execInfo.SearchAttributes.IndexedFields[sadefs.TemporalScheduledById]
s.True(hasScheduledByID)

_, hasTotalCount := execInfo.SearchAttributes.IndexedFields["TemporalInt01"]
s.False(hasTotalCount, "CHASM search attribute TemporalInt01 should not be exposed")
_, hasTotalSize := execInfo.SearchAttributes.IndexedFields["TemporalInt02"]
s.False(hasTotalSize, "CHASM search attribute TemporalInt02 should not be exposed")
}

// TODO: More tests here...
Loading