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

Fixing bugs in flatgeobuf input #37

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Fixing bugs in flatgeobuf input #37

wants to merge 9 commits into from

Conversation

e-n-f
Copy link
Collaborator

@e-n-f e-n-f commented Nov 22, 2022

No description provided.

@@ -168,17 +167,24 @@ void readFeature(const FlatGeobuf::Feature *feature, long long feature_sequence_
// https://github.com/protomaps/tippecanoe/issues/7
// check if column name is tippecanoe:minzoom, tippecanoe:maxzoom or tippecanoe:layer

FlatGeobuf::ColumnType col_type = h_column_types[col_idx];
FlatGeobuf::ColumnType col_type;
if (feature->columns() != NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How are you testing this? Last I checked the ogr2ogr writer always puts column type defs in the header.

@@ -381,11 +381,7 @@ void parse_flatgeobuf(std::vector<struct serialization_state> *sst, const char *
long long feature_sequence_id = -1;
int index_size = 0;
if (node_size > 0) {
if (!quiet) {
fprintf(stderr, "detected indexed FlatGeobuf: assigning feature IDs by sequence\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

Fine with removing this for now; FYI the rationale was that the sequence # in a spatially indexed FGB is the key useful for accessing a single feature in a "cloud-optimized" way to enable potential workflows where Tippecanoe can hold a reference to access whole OGC Features via another fetch. If I find an opportunity to actually implement something like that then maybe it deserves to be in an option sibling to --use-attribute-for-id=... like --use-fgb-seqnum-for-id

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

Successfully merging this pull request may close these issues.

2 participants