Skip to content

feat(db): support SearchDirection in QueryTransactionsParams#1536

Open
EvanYan1024 wants to merge 1 commit intohyperledger-labs:mainfrom
EvanYan1024:feat/transaction-query-search-direction
Open

feat(db): support SearchDirection in QueryTransactionsParams#1536
EvanYan1024 wants to merge 1 commit intohyperledger-labs:mainfrom
EvanYan1024:feat/transaction-query-search-direction

Conversation

@EvanYan1024
Copy link
Copy Markdown
Contributor

@EvanYan1024 EvanYan1024 commented Apr 13, 2026

Closes #1539

Summary

QueryTransactions previously hard-coded ORDER BY stored_at ASC, making it
impossible for callers to request newest-first ordering.

This PR adds a *SearchDirection field (pointer) to QueryTransactionsParams:

  • nil (default): ascending order — preserves backward compatibility with all existing callers
  • &FromLast: descending order (newest first)
  • &FromBeginning: ascending order (oldest first, explicit)

Using a pointer avoids the zero-value ambiguity (FromLast = 0), ensuring
existing code that uses QueryTransactionsParams{} continues to work unchanged.

Changes

  • driver/common.go: Add *SearchDirection field to QueryTransactionsParams
  • sql/common/transactions.go: Add transactionOrderBy() helper that defaults to ASC when direction is nil
  • dbtest/transactions.go: Add SearchDirection ASC/DESC test cases using records with distinct timestamps

Test plan

  • go test passes for memory, SQLite, and PostgreSQL backends locally
  • Unit tests verify both ASC and DESC ordering with deterministic timestamps
  • Mock SQL test (TestQueryTransactions) confirms default query uses ORDER BY stored_at ASC
  • All existing tests pass without modification (backward compatible)

@adecaro
Copy link
Copy Markdown
Contributor

adecaro commented Apr 13, 2026

Hi @EvanYan1024 , thanks for submitting this and for the effort. I'll review ASAP.
Please, open a Github Issue. Many thanks 🙏

@adecaro
Copy link
Copy Markdown
Contributor

adecaro commented Apr 13, 2026

Hi @EvanYan1024 , thanks for creating the issue.
This is actually very helpful 🙏

Please, have a look at the unit-tests. There must be something there that if fixed might solve also the integration tests.
Thanks.

@EvanYan1024 EvanYan1024 force-pushed the feat/transaction-query-search-direction branch 2 times, most recently from 4888046 to 27f5385 Compare April 13, 2026 06:42
@EvanYan1024
Copy link
Copy Markdown
Contributor Author

Hi @EvanYan1024 , thanks for creating the issue. This is actually very helpful 🙏

Please, have a look at the unit-tests. There must be something there that if fixed might solve also the integration tests. Thanks.

@adecaro Hi,I have fixed all CI errors. thanks

QueryTransactions previously hard-coded ORDER BY stored_at ASC,
making it impossible for callers to request newest-first ordering.

Add a *SearchDirection field (pointer) to QueryTransactionsParams.
When nil (zero value), the query defaults to ASC for backward
compatibility with all existing callers. Set explicitly to
&FromLast for descending or &FromBeginning for ascending.

This is consistent with the existing SearchDirection field in
QueryMovementsParams, while preserving backward compatibility.

Signed-off-by: Evan <evanyan@sign.global>
@adecaro adecaro force-pushed the feat/transaction-query-search-direction branch from 27f5385 to 6e1d50a Compare April 13, 2026 08:57
@AkramBitar AkramBitar added the enhancement New feature or request label Apr 13, 2026
// SearchDirection is the direction of the search.
// If nil, defaults to FromBeginning (ascending by stored_at) for backward compatibility.
// Set explicitly to override: &FromLast for descending, &FromBeginning for ascending.
SearchDirection *SearchDirection
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we don't need the pointer here because 0 (the default value) represents searching from last. No?

@adecaro
Copy link
Copy Markdown
Contributor

adecaro commented Apr 13, 2026

Hi @EvanYan1024 , I think we can remove the pointer and then we are good.
Thanks for the effort 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

db enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

QueryTransactionsParams lacks SearchDirection support (hard-coded ASC ordering)

3 participants