BemiHQ / BemiDB

Postgres read replica optimized for analytics
https://bemidb.com
GNU Affero General Public License v3.0
1.1k stars 21 forks source link

Fix COPY command output to local file #8

Closed sikinatm closed 2 weeks ago

sikinatm commented 2 weeks ago

Thank you for developing such a wonderful tool.
While trying it locally, I encountered an issue that I think might be a problem(may be related to https://github.com/BemiHQ/BemiDB/issues/2), and I would like to propose the following changes.

Problem

During synchronization, the process fails when attempting to read from a temporary file, resulting in an EOF (End of File) error. The error occurs at this point.

bemidb    | panic: EOF
bemidb    |
bemidb    | goroutine 1 [running]:
bemidb    | main.PanicIfError({0x27dd9a0?, 0x34c2640?}, {0x0?, 0x2673c0f?, 0x40002045f8?})
bemidb    |     /app/utils.go:14 +0x9c
bemidb    | main.(*Syncer).syncFromPgTable(0x40002161e0, 0x4000000c60, {{0x40002045b8?, 0x6?}, {0x40002045f8?, 0x0?}})
bemidb    |     /app/syncer.go:108 +0x2f4
bemidb    | main.(*Syncer).SyncFromPostgres(0x40002161e0)
bemidb    |     /app/syncer.go:46 +0x1a8
bemidb    | main.syncFromPg(0x355f440)
bemidb    |     /app/main.go:60 +0x24
bemidb    | main.main()
bemidb    |     /app/main.go:25 +0xb0

Solution

Previously, the COPY command was executed in a way that created the result file on the Postgres server:

result, err := conn.Exec(
    context.Background(),
    "COPY " + pgSchemaTable.String() + " TO '" + tempFile.Name() + "' WITH CSV HEADER",
)

I think that creating the file locally is the intended behavior, and I would like to propose the following changes to align with that.

result, err := conn.PgConn().CopyTo(
    context.Background(),
    tempFile,
    "COPY "+pgSchemaTable.String()+" TO STDOUT WITH CSV HEADER",
)
exAspArk commented 2 weeks ago

Hi @sikinatm — thanks a lot for submitting this PR! 🙌 

Do you happen to know why the process failed with Exec? Is it related to Linux and some file paths / permissions? I haven't got a chance to look deeper into the issue.

Update: Oh, I see. If Postgres is running remotely, we want to write into BemiDB's local file, not Postgres'!

exAspArk commented 2 weeks ago

This is released in v0.3.0

sikinatm commented 2 weeks ago

@exAspArk Thanks a lot for accepting my proposal!