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

geographic axes for Kyrix-S #153

Merged
merged 3 commits into from
Sep 2, 2020
Merged

geographic axes for Kyrix-S #153

merged 3 commits into from
Sep 2, 2020

Conversation

tracyhenry
Copy link
Owner

This PR allows specification of geo axes, freeing the user from manually transforming lat/lon to screen coordinates.

now you can specify the following:

ssv = {
...
layout: {
  x:{
    field: 'latitude'
  },
  y:{
    field: 'longitude'
  },
  geo:{
    level: 5,
    center: [39.5, -98.5]
  }
},
...
}

the initial viewport will be pointed to layout.geo.center at zoom level layout.geo.level. And an OpenStreet map background will be automatic rendered.

@tracyhenry tracyhenry merged commit a8a8305 into master Sep 2, 2020
@tracyhenry tracyhenry deleted the ssv_geo branch September 2, 2020 04:30
Copy link
Collaborator

@asah asah left a comment

Choose a reason for hiding this comment

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

Nice!

System.out.println(sql);
rawDbStmt.executeUpdate(sql);

sql =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add comment explaining REPLACE_ME ?


sql =
"UPDATE "
+ ssv.getRawTable()
Copy link
Collaborator

Choose a reason for hiding this comment

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

SQL injection...

suggestion: instead of using injection safe parameters (ugly pain), rename getRawTable() to add something indicating safety e.g. getRawTableSqlSafe(), and then inside the function, restrict the character set to ensure no injection, I.e. you don't care about during weird table names. Wrapping also works (e.g. sqlSafe(getRawTable()) though it requires told support to ensure that you don't forget to do this everywhere...

Copy link
Owner Author

Choose a reason for hiding this comment

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

Makes sense. Since getRawTable returns a table name we get from compiler - the validation should be done in compiler.

I think this falls into the bigger issue #15, where we need to do better validation of user spec to deal with "overly clever" developers.

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