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

filter-plugin: Inner Txn support. #1487

Merged
merged 18 commits into from
Mar 1, 2023

Conversation

winder
Copy link
Contributor

@winder winder commented Feb 24, 2023

Summary

  1. Generator switched to use SignedTxnWithAD instead of SignedTxnInBlock.
  2. Split out a matches function from SearchAndFilter to avoid duplicate code for operation types (any/all/none).
  3. Rename Search to Match for lowest level expressions.
  4. Rename FilterType to Type.
  5. Remove Filter from the Filter constants, it is in their type.
  6. There is one remaining search function in the Searcher type. It now has an option to recursively search inner transactions for a match.

Test Plan

New unit test.

Updated benchmarks.

@winder winder changed the title Filter Plugin Inner Txn support. filter-plugin: Inner Txn support. Feb 24, 2023
@winder winder added the Enhancement New feature or request label Feb 24, 2023
@codecov
Copy link

codecov bot commented Feb 24, 2023

Codecov Report

Merging #1487 (2cfe74f) into develop (ffcd97f) will increase coverage by 0.89%.
The diff coverage is 79.79%.

@@             Coverage Diff             @@
##           develop    #1487      +/-   ##
===========================================
+ Coverage    65.39%   66.28%   +0.89%     
===========================================
  Files           82       81       -1     
  Lines        11458    11260     -198     
===========================================
- Hits          7493     7464      -29     
+ Misses        3393     3226     -167     
+ Partials       572      570       -2     
Impacted Files Coverage Δ
...plugins/processors/filterprocessor/gen/generate.go 36.92% <0.00%> (ø)
...ilterprocessor/expression/numerical_expressions.go 59.01% <66.66%> (ø)
...lugins/processors/filterprocessor/fields/filter.go 77.14% <73.07%> (-2.86%) ⬇️
...gins/processors/filterprocessor/fields/searcher.go 77.50% <85.00%> (-0.28%) ⬇️
...rocessors/filterprocessor/expression/expression.go 96.72% <92.00%> (-3.28%) ⬇️
...ins/processors/filterprocessor/filter_processor.go 80.00% <100.00%> (ø)
...filterprocessor/fields/generated_signed_txn_map.go
idb/postgres/postgres.go 65.63% <0.00%> (+0.13%) ⬆️
... and 1 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@winder winder marked this pull request as ready for review February 24, 2023 19:08
@winder
Copy link
Contributor Author

winder commented Feb 27, 2023

Benchmark results

BenchmarkProcess/inner_txn_count_0
BenchmarkProcess/inner_txn_count_0-8         	 1274750	       920.6 ns/op
BenchmarkProcess/inner_txn_count_10
BenchmarkProcess/inner_txn_count_10-8        	  219909	      5759 ns/op
BenchmarkProcess/inner_txn_count_100
BenchmarkProcess/inner_txn_count_100-8       	   25620	     48310 ns/op

Comment on lines +59 to +70
type stringEqualExpression struct {
Str string
}

func (e *stringEqualExpression) Match(input interface{}) (bool, error) {
switch v := input.(type) {
case string:
return e.Str == v, nil
default:
return false, fmt.Errorf("unexpected regex search input type (%T)", v)
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously this was using a special regex expression. In benchmarks using string equals was 8-12% faster.

fields, errors := getFields(sdk.SignedTxnInBlock{}, ignoreTags)
fields, errors := getFields(sdk.SignedTxnWithAD{}, ignoreTags)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switched to SignedTxnWithAD because that is the type used for inner transactions. This way the same lookup function is used for the root transaction and the inner transactions.

Copy link
Contributor

@shiqizng shiqizng left a comment

Choose a reason for hiding this comment

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

lgtm

@winder winder merged commit fe5f2c6 into algorand:develop Mar 1, 2023
@winder winder deleted the will/filter-inner-txn branch March 1, 2023 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants