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

chore: remove deny with sqlx #4663

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

driftluo
Copy link
Collaborator

@driftluo driftluo commented Sep 25, 2024

What problem does this PR solve?

remove deny with sqlx

By the way, I think that SQL creates tables and indexes that can use IF NOT EXISTS to avoid judging whether it has been initialized. Although it is a non-standard statement, but both SQLite and Postgresql support it.

let require_init = self.is_postgres_require_init(db_config).await?;

if require_init {
self.create_tables_for_postgres().await?;
}

fn is_sqlite_require_init(db_config: &RichIndexerConfig) -> bool {
// for test
if db_config.store == Into::<PathBuf>::into(MEMORY_DB) {
return true;
}
if !db_config.store.exists() {
if let Some(parent) = db_config.store.parent() {
std::fs::create_dir_all(parent).expect("Create db directory");
}
OpenOptions::new()
.write(true)
.create(true)
.open(&db_config.store)
.expect("Create db file");
return true;

pub async fn is_postgres_require_init(
&mut self,
db_config: &RichIndexerConfig,
) -> Result<bool> {
// Connect to the "postgres" database first
let mut temp_config = db_config.clone();
temp_config.db_name = "postgres".to_string();
let uri = build_url_for_postgres(&temp_config);
let mut connection_options = AnyConnectOptions::from_str(&uri)?;
connection_options.log_statements(LevelFilter::Trace);
let tmp_pool_options = AnyPoolOptions::new();
let pool = tmp_pool_options.connect_with(connection_options).await?;
// Check if database exists
let query =
SQLXPool::new_query(r#"SELECT EXISTS (SELECT FROM pg_database WHERE datname = $1)"#)
.bind(db_config.db_name.as_str());
let row = query.fetch_one(&pool).await?;

Check List

Tests

  • Unit test
  • Integration test

Release note

None: Exclude this PR from the release note.

@driftluo driftluo marked this pull request as ready for review September 25, 2024 08:09
@driftluo driftluo requested a review from a team as a code owner September 25, 2024 08:09
@driftluo driftluo requested review from doitian and eval-exec and removed request for a team September 25, 2024 08:09
eval-exec
eval-exec previously approved these changes Sep 25, 2024
zhangsoledad
zhangsoledad previously approved these changes Sep 25, 2024
@zhangsoledad zhangsoledad added this pull request to the merge queue Sep 25, 2024
@zhangsoledad zhangsoledad removed this pull request from the merge queue due to a manual request Sep 25, 2024
.await
.map_err(|err| Error::DB(err.to_string()))?;

Ok(row_lock.get::<bool, _>(0) || row_type.get::<bool, _>(0))
Ok(row_lock.get::<i64, _>(0) == 1 || row_type.get::<i64, _>(0) == 1)
Copy link
Collaborator

@eval-exec eval-exec Sep 26, 2024

Choose a reason for hiding this comment

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

Perhaps we can check and return early with Ok(row_lock.get::<i64, _>(0) == 1) after immediately fetching row_lock after L220 ?

@driftluo driftluo added this pull request to the merge queue Sep 26, 2024
@@ -215,7 +215,7 @@ async fn script_exists_in_output(
"#,
)
.bind(script_id)
.fetch_one(&mut *tx)
.fetch_one(tx.as_mut())
.await
.map_err(|err| Error::DB(err.to_string()))?;

Copy link
Collaborator

Choose a reason for hiding this comment

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

How about move row_lock.get::<i64, _>(0) == 1 checking in this line?

Merged via the queue into nervosnetwork:develop with commit f8f1bbe Sep 26, 2024
34 checks passed
@driftluo driftluo deleted the remove-deny-with-sqlx branch September 26, 2024 03:57
@15168316096
Copy link
Contributor

@driftluo
I understand that this pr may be two things to avoid: one is to pre-judge the secondary creation of tables and indexes, and to use sqlx and pgpool to optimize asynchronous queries.
The function points of verification should be,

  1. The function of rich-indexer returns to normal
  2. Query optimization of slow queries in pgpool asynchronous mode
    cc @gpBlockchain

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.

4 participants