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

Added bigquery dialect and Gorm's BigQuery driver as dependency #238

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

Conversation

pn03
Copy link

@pn03 pn03 commented Jan 12, 2025

References

Issue(s): #196

Description

This PR adds support to work with BigQuery database using Gorm's BigQuery driver v1.2.0

Data Source Name for BigQuery contains project-id, location, dataset.
Among these location is optional.
Also options contains various optional values in query string format.

DSN = "bigquery://{name}?{options}"

name = project_id[/location_id]/dataset_id
options = option1=value1&option2=value2

@pn03 pn03 changed the title added bigquery dialect and module as dependency Added bigquery dialect and Gorm's BigQuery driver as dependency Jan 12, 2025
@System-Glitch System-Glitch self-requested a review January 13, 2025 09:27
@System-Glitch System-Glitch self-assigned this Jan 13, 2025
@System-Glitch System-Glitch added the feature request Request for new feature implementation label Jan 13, 2025
@coveralls
Copy link

Pull Request Test Coverage Report for Build 12735576880

Details

  • 0 of 5 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.07%) to 97.424%

Changes Missing Coverage Covered Lines Changed/Added Lines %
database/dialect/bigquery/bigquery.go 0 5 0.0%
Totals Coverage Status
Change from base Build 12292911853: -0.07%
Covered Lines: 6429
Relevant Lines: 6599

💛 - Coveralls

@System-Glitch
Copy link
Member

This PR looks good to me. I think it's on GORM's end, but I can't make it work at all. The query arguments (?) cause errors:

sql: expected 0 arguments, got 2

Query:

SELECT * FROM `some_table` WHERE `id` = ? ORDER BY `some_table`.`id` LIMIT ?

Repo method:

func (r *SomeRepo) GetByID(ctx context.Context, id uint) (*model.SomeModel, error) {
	var result *model.SomeModel
	db := session.DB(ctx, r.DB).Where("id", id).First(&result)
	return result, errors.New(db.Error)
}

Have you managed to make it work on your end?

@System-Glitch
Copy link
Member

Please add the following:

// New create a new connection pool using the settings defined in the given configuration.
//
// In order to use a specific driver / dialect ("mysql", "sqlite3", ...), you must not
// forget to blank-import it in your main file.
//
//	import _ "goyave.dev/goyave/v5/database/dialect/mysql"
//	import _ "goyave.dev/goyave/v5/database/dialect/postgres"
//	import _ "goyave.dev/goyave/v5/database/dialect/sqlite"
//	import _ "goyave.dev/goyave/v5/database/dialect/mssql"
//	import _ "goyave.dev/goyave/v5/database/dialect/clickhouse"
+ //	import _ "goyave.dev/goyave/v5/database/dialect/bigquery"
func New(cfg *config.Config, logger func() *slog.Logger) (*gorm.DB, error) {

@pn03
Copy link
Author

pn03 commented Jan 13, 2025

Hey @System-Glitch I'll add the import comment.

Regarding the query with placeholders ?.
Based on documentation I found out that named parameters need to be supplied in SQL formatted query : Google's BigQuery Client Package

Any way I'll check more and get back to you on this.

@System-Glitch
Copy link
Member

I think I found the issue. This driver is unusable in its current state, let's wait for that issue to be solved before merging this.

@pn03
Copy link
Author

pn03 commented Jan 13, 2025

Sure no worries

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Request for new feature implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants