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

PostGIS driver needs to handle nil tag values #67

Closed
ARolek opened this issue Aug 6, 2016 · 6 comments
Closed

PostGIS driver needs to handle nil tag values #67

ARolek opened this issue Aug 6, 2016 · 6 comments
Labels

Comments

@ARolek
Copy link
Member

ARolek commented Aug 6, 2016

The following error is being thrown when a column is nil.

Error Getting VTile: Error Getting VTileLayer: Unsupported type for value(<nil>) with key(name) in tags for feature {Feature: 2, GEO: LINESTRING (796342.3896334589 6.585025839381214e+06,796372.5572154642 6.585048134886125e+06,796461.2009259841 6.585103944357122e+06,796553.1619573263 6.585152392708984e+06,796612.0944957548 6.585178334092038e+06), Tags: map[name:<nil>]}.
@gdey gdey closed this as completed in b8a5b8c Aug 6, 2016
@gdey gdey reopened this Aug 6, 2016
@gdey
Copy link
Member

gdey commented Aug 6, 2016

So, this bug is turning out to be a bit deeper then it first appears. The problem isn't jsut the nil, but also other unknow types. That the provider can stuff into the tags map. So, the fix really need to be at the provider level. Basically, the provider need to convert any custom types it may expect into types that can be encoded in the basic type. Most likely this will be converting it to a string.

I should probally error if there are tag values that are not supported when the feature is being created, and not when it's being encoded.

@ARolek
Copy link
Member Author

ARolek commented Aug 7, 2016

@gdey This still might be two different issues.

The "nil" bug has to do when a row does not have a value. This was showing up when custom SQL was being used that returned the id, geom and name fields and the name field was nil sometimes. Skipping nil values could work, though we may find an use case where having the name key with an empty value desirable to maintain consistency with the tags from feature to feature within a layer.

The second issue is what you're describing where custom types need to be mapped to a supported format. I think a string would work.

@ARolek ARolek added the bug label Aug 15, 2016
@kalxas
Copy link

kalxas commented Aug 15, 2016

With the previous configuration I was using this custom SQL without issues:
https://github.com/terranodo/geonode-live/blob/71ac7f2726612c2c13e0ab8aa247b74e52556e0b/conf/tegola/config.toml#L23

@ARolek
Copy link
Member Author

ARolek commented Aug 15, 2016

@kalxas I'm about to shift into this bug. Thanks for providing the old config.

@ARolek
Copy link
Member Author

ARolek commented Aug 15, 2016

@gdey I have a solution that has Bonn rendering again. Essentially if we add a check for "nil" in our feature tag switch here and essentially do nothing, the key will be encoded but the value will be nil. For example:

        for i, v := range valueMap {
            switch tv := val.(type) {
            default:
                return tags, fmt.Errorf("Value (%[1]v) of type (%[1]T) for key (%[2]v) is not supported.", tv, key)

            case nil:
                //  nil values can be ignored. the key will still be encoded.

            case string:
                ...

I do like the idea of keeping the key there even though the value is nil as it will keep the feature attributes consistent from feature to feature. Do you see any issues with this?

@ARolek
Copy link
Member Author

ARolek commented Aug 15, 2016

I did some more research and it looks like currently the spec does not support null value types so the key should be ignored. See the conversation here.

gdey added a commit that referenced this issue Aug 16, 2016
@gdey gdey mentioned this issue Aug 16, 2016
@gdey gdey closed this as completed in #72 Aug 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants