Bug in QWATCH - client is not notified when match not found for WHERE query clause

[psrvere]

Found the bug while writing integration test for QWATCH Where clause.

Example query

query := "SELECT $key, $value WHERE '$value.score' >= 90 AND $key like 'player:*'"

For any score less than 90, processWatchEvent function will return without any response

Following is the relevant snippet from processWatchEvent function:

		// Check if the key matches the regex
		if query.Where != nil {
			matches, err := sql.EvaluateWhereClause(query.Where, sql.QueryResultRow{Key: event.Key, Value: event.Value}, make(map[string]jp.Expr))
			if err != nil || !matches {
				return true
			}
		}

		w.updateQueryCache(query.Fingerprint, event)

		queryResult, err := w.runQuery(&query)
		if err != nil {
			w.logger.Error(err.Error())
			return true
		}

		w.notifyClients(&query, clients, queryResult)
		return true

For matches = false, function returns without notifying client.

Suggestion to fix the bug - don't exit if matches is false

		// Check if the key matches the regex
		if query.Where != nil {
			_, err := sql.EvaluateWhereClause(query.Where, sql.QueryResultRow{Key: event.Key, Value: event.Value}, make(map[string]jp.Expr))
			if err != nil {
				return true
			}
		}

		w.updateQueryCache(query.Fingerprint, event)

		queryResult, err := w.runQuery(&query)
		if err != nil {
			w.logger.Error(err.Error())
			return true
		}

		w.notifyClients(&query, clients, queryResult)
		return true

This will add event where matches = false to query cache and notify client.

[psrvere]

@JyotinderSingh - please confirm if this is bug is correct. Will raise fix PR if you are fine with the solution.

[JyotinderSingh]

This is expected behaviour. If the no key matches the filter condition we would return an empty result (same as regular sql).

[psrvere]

Got it.

Any thoughts on why tests get stuck on the second test case. This gets fixed if notifyClients function is called by making above change.

tests := []struct {
		key             string
		value           string
		expectedUpdates [][]interface{}
	}{
		{
			key:   "player:1",
			value: `{"name":"Alice","score":100}`,
			expectedUpdates: [][]interface{}{
				{[]interface{}{"player:1", map[string]interface{}{"name": "Alice", "score": float64(100)}}},
			},
		},
		{
			key:   "player:2",
			value: `{"name":"Bob","score":80}`,
			expectedUpdates: [][]interface{}{
				{[]interface{}{"player:1", map[string]interface{}{"name": "Alice", "score": float64(800)}}},
			},
		},
		{
			key:   "player:3",
			value: `{"name":"Charlie","score":90}`,
			expectedUpdates: [][]interface{}{
				{[]interface{}{"player:1", map[string]interface{}{"name": "Alice", "score": float64(100)}},
					[]interface{}{"player:3", map[string]interface{}{"name": "Charlie", "score": float64(90)}}},
			},
		},
		{
			key:   "player:4",
			value: `{"name":"David","score":95}`,
			expectedUpdates: [][]interface{}{
				{[]interface{}{"player:1", map[string]interface{}{"name": "Alice", "score": float64(100)}},
					[]interface{}{"player:3", map[string]interface{}{"name": "Charlie", "score": float64(90)}},
					[]interface{}{"player:4", map[string]interface{}{"name": "David", "score": float64(95)}}},
			},
		},
	}

This is probably because notifyClients calls sendWithRetry which sends proper response.

func (w *QueryManager) sendWithRetry(query *sql.DSQLQuery, clientFD int, data []byte) {
	fmt.Println("In QueryWatcher:sendWithRetry")
	maxRetries := 20
	retryDelay := 20 * time.Millisecond

	for i := 0; i < maxRetries; i++ {
		_, err := syscall.Write(clientFD, data)
		if err == nil {
			return
		}

[psrvere]

I think when we return true for matches = false, we exit sync.Map.Range iteration without executing notifyClient function. We need to execute this function with empty bytes. @JyotinderSingh

[JyotinderSingh]

This is still expected behaviour. The where clause evaluates for every key update in the database, irrespective of whether it affects a query or not. The where clause is used to check whether a key update affects a given key.

This means, that if we have one QWATCH query filtering for keys that match a pattern match:100:user:*, and another key in the database (say session_token_user_200) gets updated - this update should not have any effect on the QWATCH query, nor should the client listening on the query receive any updates. Otherwise, we would execute every watched query every time a key gets set/updated/deleted, which is not desirable.

As for your test case, the tests should expect no updates to be delivered if a key which is unrelated to the query is updated.

[psrvere]

That makes sense. Thanks for the explanation. Closing the issue.