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

PoC of generated code for type-safe SQL #1

Draft
wants to merge 3,375 commits into
base: master
Choose a base branch
from
Draft

Conversation

sreuland
Copy link
Owner

@sreuland sreuland commented Dec 13, 2021

PR Checklist

PR Structure

  • This PR has reasonably narrow scope (if not, break it down into smaller PRs).
  • This PR avoids mixing refactoring changes with feature changes (split into two PRs
    otherwise).
  • This PR's title starts with name of package that is most changed in the PR, ex.
    services/friendbot, or all or doc if the changes are broad or impact many
    packages.

Thoroughness

  • This PR adds tests for the most critical parts of the new functionality or fixes.
  • I've updated any docs (developer docs, .md
    files, etc... affected by this change). Take a look in the docs folder for a given service,
    like this one.

Release planning

  • I've updated the relevant CHANGELOG (here for Horizon) if
    needed with deprecations, added features, breaking changes, and DB schema changes.
  • I've decided if this PR requires a new major/minor version according to
    semver, or if it's mainly a patch change. The PR is targeted at the next
    release branch if it's not a patch change.

What

PoC of using a code generator library/tool such as go-jet against schema of rdbms tables to enable type safe go code for SQL statements rather than hardcoded SQL snippets. Following precedence of library JOOQ Manifesto..no abstraction. Determine how realistic it may or may not be to use a tool like this. Two query methods were refactored to use Gojet generated code for SQL, in order to see the overall footprint:

history.AccountEntriesForSigner()
history.FeeStats()

to run the poc:

$ git clone https://github.com/sreuland/go.git; cd go
$ git checkout master 
$ go install ./services/horizon/...
$ git checkout generated_db_sql_poc
# have postgres instance on localhost:5432 available with latest Horizon schema loaded:
$ createdb -O postgres -U postgres -h localhost horizon
$ horizon db init --db-url postgres://localhost/horizon?sslmode=disable 
$ ./gogenerate.sh
$ go test ./...

Why

hardcoded SQL statement snippets can be difficult to maintain over time, especially if they are concat'd to other snippets, this PoC explores whether generated go code representing rdbms tables enables more efficient SQL coding.

Known limitations

there are many custom extensions in Postgres that the Gojet library/tool may not map yet and are being used in horizon, resulting in some percentage of SQL statements that can't be converted to codegen'd, or at least require filing a ticket/request with Gojet first to get support. For example, filed one such ticket with Gojet, #106 for lack of support for some postgres aggregate functions, repo is active, author responded fairly soon, within a couple weeks added support.

bartekn and others added 30 commits August 27, 2021 16:01
…tellar#3863)

Adds liquidity_pools to TruncateIngestStateTables to clear it during state
rebuild.
clients/horizonclient: Adding some basic tests for clients/horizonclient.LiquidityPool(s)
…rsion

services/horizon: bump check_release_hash to 1.16 to match circle and jenkins
Added a new Protocol 18 and missing codes from previous protocol upgrade. Added
a test to ensure return codes of all specific operation types are implemented.
…#3868)

This commit adds a _happy path_ integration test for liquidity pool. It includes
LP creation, deposit, withdrawal and deletion. Then it checks if operations,
effects and trades are ingested correctly. Finally, all bugs found by the test
has been fixed.
…ions (stellar#3880)

When a liquidity pool operation belongs to a failed transaction we
cannot easily retreive the liquidity pool (on failure, the transaction
metadata won't include liquidity pool changes). In fact, we may not be
able to retreive the liquitity pool at all (e.g. if the liquidity pool
id of the operation is incorrect and the transaction failed for that
reason).

Thus, we make the asset optional in the /operation API responses.

When an operation belongs to a failed transaction we omit the asset.
Also, we set the amount in `reserves_received`, `reserves_deposited`
and `shares_recieved` to 0, which is consistent with a failure (in
that case no shares nor or reserves change).
* Enforce asset ordering in xdr.NewPoolId to prevent blunders

* have xdr.NewPoolId Implicitly order assets

* Add helpers for txnbuild/LiquidityPool(Withdraw|Deposit) to implicitly sort assets

* Add comment about prices

* review feedback

* use AssetAmount tuple in NewLiquidityPoolDeposit/Withdraw

* No implicit ordering of assets, it gets weird with inverting the minPrice

* xdr.PoolId should reject incorrect ordering

* Fix tests

Co-authored-by: Alfonso Acosta <[email protected]>
This commit adds Liquidity Pools to asset stats.

Assets stats should include all data from all ledger entries so the sum of
balances equals the sum of circulating supply of an issued asset.
Fix trade and trade aggregation tests
…umpTransaction, GenericTransaction (stellar#3897)

Implement `encoding.TextMarshaler` and `encoding.TextUnmarshaler` in `Transaction`, `FeeBumpTransaction`, `GenericTransaction`.

Transaction, FeeBumpTransaction, and GenericTransaction all have well defined formats for serialization as strings. It would be convenient if these types were automatically serializable via encoders such as the `encoding/json` package.

Constructors for GenericTransaction are added so that code building Transaction and FeeBumpTransaction can get a GenericTransaction for those built transactions, for setting in a field that can hold both, in a type supporting serialization.
…r to indicate support (stellar#3891)

Allow the `Fund` function to submit a fund request to any horizon instance, not just the instance defined by `horizonclient.DefaultTestNetClient`, and rely on the horizon instance responding with a 404 as an indicator that funding is not supported.

The only way to construct a `horizonclient.Client` that supports the `Fund` operation is to use and mutate the `horizonclient.DefaultTestNetClient`. This is unnecessarily restrictive because a developer may wish to use the `Fund` operation with a non-default client that they have constructed themselves, or they may wish to use the `Fund` operation with their own private or standalone test network.
…edger update methods (stellar#3900)

This commit splits `UpdateLedgerState` into `UpdateCoreLedgerState` and
`UpdateHorizonLedgerState` which update Core and Horizon ledger state
separately. It also changes `Tick` frequency to 5 seconds.

If Stellar-Core is unresponsive the call to Stellar-Core `/info` endpoint  will
be cancelled after 10 seconds but it will also return entire `UpdateLedgerState`
method. This will make Horizon ledger stats out of date which can affect
streaming (because streams are updated only when Horizon ledger in
`ledger.Status` is incremented).

Known limitations: with this change it's possible that Horizon root can report
Core ledger sequence behind Horizon.
2opremio and others added 28 commits December 11, 2021 09:38
and fix the liquidity pools and claimable balances templates as a result.
…type filter performance (stellar#4149)

Add trade_type column which is an enum value that represents whether the trade occurs on the orderbook or against a liquidity pool. The trade_type column is populated during ingestion. This commit also adds an index on the trade_type column to make queries for liquidity pool trades fast.
…4151)

As far as I have seen, the stream object is never used in a
mult-goroutine context.

Also, we were not properly counting the 'hello' event from the preamble.
Due to the error logic (which counts the sent events) this could cause
an error right after the 'hello' event to be incorrectly rendered as JSON.
* Use the latest protocol version for all tests.
* Set the protocol to the latest version if not specified explicitly.
* Rename tests according to their content, stop using protocol numbers.
…ol type (stellar#4158)

* Add method to ingest.Change that returns liquidity pool type
…ng middleware (stellar#4163)

Rate limiting is applied on horizon http requests via a middleware. However, for streaming requests, rate limiting is also applied on each execution of the event handler.
Add ToXDR, Hash, HashHex to GenericTransaction, copying the semantics of those same functions already available on FeeBumpTransaction and Transaction.

I'm writing some code that needs to get the hash of a transaction, irrespective of whether it is a fee bump or a standalone transaction. A GenericTransaction wraps a transaction envelope, and a transaction envelope always has a hash, so it seems appropriate to introduce this functionality here.
  * Remove `TransactionParams.EnableMuxedAccounts`
  * Remove `TransactionFromXDROptionEnableMuxedAccounts`
  * Remove `FeeBumpTransactionParams.EnableMuxedAccounts`
  * Remove parameter `withMuxedAccounts bool` from all methods/functions.
  * Remove `options ...TransactionFromXDROption` parameter from `TransactionFromXDR()`
  * Rename `SetOpSourceMuxedAccount()` to (pre-existing) `SetOpSourceAccount()` which now accepts
    both `G` and `M` (muxed) account strkeys.
…rings. (stellar#4167)

Use xdr.Price to represent prices in txnbuild instead of strings
…r#4150)

Update cursor in Stellar-Core to latest ingested ledger when local ingestion is
behind the cluster.

If there is a DB ingesting instance more than one ingesting instance: either
Captive-Core or DB connected to another Stellar-Core, it's possible that the
cursor will be updated very rarely. This will make the Stellar-Core DB size
bloat as the old ledger will not be removed.
Add AddSignatureDecorated to FeeBumpTransaction.

The function exists on Transaction but not on FeeBumpTransaction. I am writing some code that needs to be able to add decorated signatures to fee bump transactions in the same way we can add them to regular transactions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.