Skip to content

Support type="double" for columns#2488

Open
amandasaurus wants to merge 1 commit into
osm2pgsql-dev:masterfrom
amandasaurus:pg-num-types
Open

Support type="double" for columns#2488
amandasaurus wants to merge 1 commit into
osm2pgsql-dev:masterfrom
amandasaurus:pg-num-types

Conversation

@amandasaurus
Copy link
Copy Markdown
Contributor

osm2pgsql already supports real, which is single precision floating point in PostgreSQL. This patch adds type = "double" for PostgreSQL double precision columns.

Users were still able to add this type of column with sql_type = "double precision"} (e.g. postpass lua). But this makes it easier.

Lua uses double precision floating point types by default.

This PR is in draft, because I was unable to run the tests. I have successfully tested this code locally. But I am not confident enough with how the interacts with lua. I would appreciate someone confirming it's OK.

Documentation for this is on my osm2pgsql-website fork

Copy link
Copy Markdown
Collaborator

@joto joto left a comment

Choose a reason for hiding this comment

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

We need tests for this, I suggest in tests/test-output-flex-types.cpp . If you can't run the tests locally, you'll see the results of the test run in your branch on Github.

Comment thread src/flex-table-column.cpp Outdated
{"int8", table_column_type::int8},
{"bigint", table_column_type::int8},
{"real", table_column_type::real},
{"double", table_column_type::double_},
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

To avoid the awkward C++ name double_, maybe name it double_precision? double is okay for the Lua side, I am only talking about the C++ side here.

Comment thread src/flex-write.cpp Outdated
throw fmt_error("Invalid type '{}' for real column.",
lua_typename(lua_state, ltype));
}
} else if (column.type() == table_column_type::double_) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Because this is just a copy of the code for the real type (except the one place in the error message), I think we can do both in one if-condition with "type=real or type=double" and change the error message to "...for real/double column".

It already supports `real`, which is single precision floating point in
PostgreSQL. This patch adds `type = "double"` for PostgreSQL `double
precision` columns
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.

2 participants