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.

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

Setting Up a Repo Function Which Leverages Raw() to Push Process of Grouping Bets Table to Create a Market:User Map #352

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Setting Up a Repo Function Which Leverages Raw() to Push Process of Grouping Bets Table to Create a Market:User Map #352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
48 changes: 48 additions & 0 deletions backend/repository/bets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package repository
Copy link
Collaborator

Choose a reason for hiding this comment

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

I still find this term repository to be confusing. I've only ever heard it used to refer to a code base not a wrapper for a database. Do you have any examples of it being used this way?

Copy link
Member Author

Choose a reason for hiding this comment

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

Stated this on discord, putting it here as well for transparency.

https://github.com/openpredictionmarkets/socialpredict/pull/352/files ... Here is some more background information on the concept of a repository pattern, specifically in golang. https://github.com/jorzel/go-repository-pattern ... related to this, is the concept of Domain Driven Design (DDD). We are already following a DDD in that our business logic is in Golang and our Application logic is in React. So what the repository pattern is, is an Infrastructure Layer which basically says, in an authoritarian way, "this is how you interact with the database," ... with the idea that, we're designing efficiency from the start, reducing refactoring down the line that we may need to deal with if we consider that our bet table should grow geometrically with the number of users. E.g. the way we are typically doing queries now, is we are pulling the entire bet table, and then having Golang run through that table and having faith that Golang is fast enough to deal with this. So while Golang is extremely powerful, my concern would be if we have a large number of users simultaneously, an event, let's say in the case of Kenyon, election day, and all of those users are pulling the entire table again and again with different functions they are doing, that becomes an I/O problem and a memory problem. Though admittedly as we discussed, I don't know how much of a memory problem that will become. Also admittedly, we could just tell users, "you need to buy more hardware for the time being."


type BetsRepository struct {
db Database
}

func NewBetsRepository(db Database) *BetsRepository {
return &BetsRepository{db: db}
}

func (repo *BetsRepository) FirstTimeBets() (int64, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not really clear on what this is supposed to return/do? Some of my follow on comments are trying to figure that out.

Can you write a comment explaining what this function is supposed to do?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. The FirstTimeBets() function is meant to quickly and efficiently count all first time bets on a particular market.

This number, an int64 is then multiplied by the https://github.com/openpredictionmarkets/socialpredict/blob/main/backend/setup/setup.yaml#L16 ... initialBetFee amount to calculate the total number of first time bet fees obtained from the market.

This can be used either in payment to the market maker upon resolution of the market, or within the sitewide stats calculation api which helps ensure that no untracked money is created in the system.

// Define a struct to hold the query result
var bets []struct {
MarketID string
Username string
}

// Execute raw SQL query to group by both market_id and username
// gorm GROUP does not allow two inputs, so this has to be a raw function
result := repo.db.Raw(`
SELECT market_id, username
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we did SELECT DISTINCT .... would that return the desired result?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't believe it does, I remember trying that, would have to go back and check this. I'm trying to get a COUNT value which we can then, "Scan" (using Gorm terminology) into an int64.

FROM bets
GROUP BY market_id, username
`).Scan(&bets)

if result.Error() != nil {
return 0, result.Error()
}

usersByMarket := make(map[string][]string)

// create map of market_id's and users for all bets
Comment on lines +30 to +32
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd switch the comment and the map making lines, and condense them to a single block.

// comment here
usersByMarket:= ....
for _, bet ...

This logic is all together so grouping them under the comment makes it clear that these go together.

for _, bet := range bets {
if usersByMarket[bet.MarketID] == nil {
// initialize the slice
usersByMarket[bet.MarketID] = []string{}
}
usersByMarket[bet.MarketID] = append(usersByMarket[bet.MarketID], bet.Username)
}

// Count total first-time bets (users across markets)
var totalFirstTimeBets int64
for _, users := range usersByMarket {
totalFirstTimeBets += int64(len(users))
}
Comment on lines +41 to +45
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this counting unique users per market for all markets? i.e. double counts of users are permitted as long as users are double counted in different markets?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct!

The reason being, as described above ... https://github.com/openpredictionmarkets/socialpredict/pull/352/files#r1817878774

We're trying to calculate fees.


return totalFirstTimeBets, nil
}
27 changes: 27 additions & 0 deletions backend/repository/database.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package repository

import "gorm.io/gorm"

type Database interface {
Scan(dest interface{}) Database
Raw(sql string, values ...interface{}) Database
Error() error
}

type GormDatabase struct {
DB *gorm.DB
}

func (g *GormDatabase) Scan(dest interface{}) Database {
g.DB = g.DB.Scan(dest)
return g
}

func (g *GormDatabase) Raw(sql string, values ...interface{}) Database {
g.DB = g.DB.Raw(sql, values...)
return g
}

func (g *GormDatabase) Error() error {
return g.DB.Error
}
Loading