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

marking most functions as PARALLEL SAFE #218

Closed

Conversation

tsl-karlp
Copy link
Contributor

Feedback appreciated. :) . I didn't mark the upgrade function as PARALLEL SAFE.

Not sure if I got the tests correct, too.

@elemoine
Copy link
Contributor

Aggregates could probably be marked PARALLEL SAFE as well.

One issue is that, in their current forms, the changes break the compatibility with PostgreSQL < 9.6. PostGIS pre-processes the postgis.sql.in files to address that problem.

@tsl-karlp
Copy link
Contributor Author

@elemoine : Is compatibility with PostgreSQL < 9.6 a requirement for pointcloud?

@elemoine
Copy link
Contributor

@elemoine : Is compatibility with PostgreSQL < 9.6 a requirement for pointcloud?

I think it would be a shame if we did not support PostgreSQL < 9.5 just because of PARALLEL SAFE.

@elemoine
Copy link
Contributor

@tsl-karlp
Copy link
Contributor Author

Unfortunately, this was the smallest lift for us, given our time. Feel free to take over. :)

@tsl-karlp tsl-karlp force-pushed the feature/parallelization branch 2 times, most recently from 5ec05a7 to 0c8f511 Compare June 17, 2018 21:44
@tsl-karlp
Copy link
Contributor Author

@elemoine : I brought over some code from postgis, which detects PG versions at the ./configure phase, and autoconf takes care of the rest. All the tests pass.

Is there something missing from this PR?

@elemoine
Copy link
Contributor

Thanks for the work on this @tsl-karlp. This is great. I'll have a look at your patch next week.

@@ -44,7 +44,7 @@ if test "x${GITCMD}" = "xyes" -a "x${DOTGITDIR}" = "xyes"; then
else
GIT_COMMIT_HASH=
fi
POINTCLOUD_VERSION="$(cat Version.config)$GIT_COMMIT_HASH"
POINTCLOUD_VERSION="$(cat Version.config)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason for changing this line?

PGSQL_MINOR_VERSION=`$PG_CONFIG --version | sed 's/[[A-Za-z ]]*//' | cut -d. -f2 | sed 's/[[^0-9]]//g'`
dnl the final version. This is to guard against user error...
PGSQL_MAJOR_VERSION=`$PG_CONFIG --version | sed 's/[[A-Za-z ]]*//' | cut -d. -f1 | sed 's/[[^0-9]]//g'`
PGSQL_MINOR_VERSION=`$PG_CONFIG --version | sed 's/[[A-Za-z ]]*//' | cut -d. -f2 | sed 's/[[^0-9]]//g'`
Copy link
Contributor

Choose a reason for hiding this comment

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

Trailing white spaces have been added here. They are to be removed again.


AC_MSG_RESULT([checking PostgreSQL version... $PGSQL_FULL_VERSION])

POSTGIS_PGSQL_VERSION="$PGSQL_MAJOR_VERSION$PGSQL_MINOR_VERSION"
Copy link
Contributor

Choose a reason for hiding this comment

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

PostGIS uses the POSTGIS_ prefix because it's PostGIS :) Let's not use the POSTGIS_ prefix in the Pointcloud extension.

@tsl-karlp
Copy link
Contributor Author

Superseded by #227 .

@tsl-karlp tsl-karlp closed this Aug 21, 2018
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.

None yet

2 participants