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

Support performing operations on tables in multiple schemas #1116

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

hfazai
Copy link
Contributor

@hfazai hfazai commented Dec 11, 2020

Fixes #145
Improves #805
This PR is to support shemas creation,cross-joins from different schemas and make selects from different schemas.

/** author is the table Author but in schema1 */
val author = Author.withSchema(schema1)

Some examples of usage

/** create tables in schemas */
val author = Author.withSchema(schema1)
val book = Book.withSchema(schema2)
SchemaUtils.create(author)
SchemaUtils.create(book)

/** insert{} */
author.insert {
    it[Author.name] = "author2-name"
}

/** test inner-joins from different schemas  */
(author innerJoin book).slice(Book.id, Author.id).selectAll().forEach {
    println("${it[Author.id]} wrote ${it[Author.id]}")
}

/** test cross-joins from different schemas  */
/** You can also use author or Author and book or Book in slice and to access results  */
(author crossJoin book).slice(Book.id, Author.id).selectAll().forEach {
    println("${it[Author.id]} wrote ${it[Book.id]}")
}

/** update table with schema. */
author.update({ Author.name eq "author1" }) {
    it[Author.name] = "author"
}

/** delete from table with schema. */
author.deleteWhere {
    Author.name eq "hichem"
 }
    
object Author : IntIdTable("author") {
    val name = varchar("name", 20)
}

object Book : Table("book") {
    val id = integer("id")
    val authorId = reference("authorId", Author).nullable()

    override val primaryKey = PrimaryKey(id)
}

@hfazai hfazai marked this pull request as draft December 11, 2020 01:45
@hfazai hfazai marked this pull request as ready for review December 12, 2020 23:43
@hfazai hfazai marked this pull request as draft December 13, 2020 00:05
@hfazai hfazai marked this pull request as ready for review February 13, 2021 17:29
@Tapac
Copy link
Contributor

Tapac commented Apr 19, 2021

Hello, @hfazai !
Thank you for PR. I see a few problems here:

  1. I think that approach with cloning table each time you call withSchema function is quite expensive as it uses reflection and if it is used in some "hot" place the user will face significant performance problem. What do you think about adding some simple LRU cache for table + scheme combination?

  2. AFAIK some users already mimic schemes by adding them into table names like FooTable : Table("A.Foo"). Those users can't use withSchema as it will return NewSchema.A.Foo as a new identity for a table.

  3. I'm against of changing metadata.getColumns(databaseName, currentScheme, "%", "%") to metadata.getColumns(null, null, "%", "%") as I know few projects with thousands of schemas/catalogs and such change will make extracting metadata for a single table a very long operation.
    I guess it's ok to group table in override fun columns(vararg tables: Table): Map<Table, List<ColumnMetadata>> { by schema and load data per-schema. In most cases, it still remains as a single request.

Also, please add a test with functions on columns from different schemas just to be sure that it works as expected.

@Tapac Tapac self-requested a review April 21, 2021 07:49
Copy link
Contributor

@Tapac Tapac left a comment

Choose a reason for hiding this comment

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

@hfazai
Copy link
Contributor Author

hfazai commented Apr 26, 2021

Hi @Tapac ,

Thanks for the changes request.

  1. In the begining I thought of providing a simple way to access columns of a table in a specific schema that's why I used reflection. But yeah, you're right, it will cause performance problems in such cases. LRU cache idea seems good to me 👍 .

I also agree with (2) and (3)

@hfazai hfazai requested a review from Tapac June 8, 2021 21:21
@hfazai
Copy link
Contributor Author

hfazai commented Jun 13, 2021

@Tapac
Done.
Feel free to review the PR.

@nhalase
Copy link

nhalase commented Apr 5, 2022

Any update on this?

@dakriy
Copy link
Contributor

dakriy commented Nov 9, 2022

Hey, I have a multi schema use case that would be easier to manage with this feature. Is any help needed with this?

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

Successfully merging this pull request may close these issues.

PostgreSQL: Referencing data on another schema
4 participants