Skip to content

Conversation

Nunalves
Copy link

@Nunalves Nunalves commented Nov 12, 2016




/**
Copy link
Member

Choose a reason for hiding this comment

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

Good idea. I'll accept this function

end sprintf;


/**
Copy link
Member

Choose a reason for hiding this comment

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

Since all the code is in one PR I'll accept this but remove this function. It's already handled in standard sprintf

@martindsouza
Copy link
Member

@Nunalves I appreciate you submitting the code. I don't think we'll be implementing this for a few reasons:

  1. Requires that user has privilege to create types. In a lot of cases DBAs don't grant that.
  2. It's not really sprintf format as I'd like to keep with how it works
  3. We just implemented oos_util_string.multi_replace which may accomplish the same thing (just different syntax.

Let me know if you're ok with these reasons or if I'm missing something.

@Nunalves
Copy link
Author

@martindsouza i appreciate you getting back to me to check my opinion on this.
It is the same functionality in a different implementation, and it does require less permissions. But the comma delimited string is not the same as having an array of values. In the real world it will lead to a lot of concatenation happening in order to build the p_replace_str string with comma separated key/value pairs and manual concatenation is what we want to avoid in the first place by using the function.
I do prefer my implementation, i think it is easier to use. I'm already using mine heavily and happily in production :)

@martindsouza
Copy link
Member

@Nunalves all very good points and I hadn't considered the concatenation issue. Any idea on how to get around the schema level type option? This is one of my biggest concerns as I'd like to avoid any restrictions from developers using this.

One idea I have is what if we overload oos_util_string.multi_replace that takes in an array instead of a string. Something like this:

function multi_replace(
    p_str in varchar2,
    p_find_str in oos_util.tab_vc2,
    p_replace_str in oos_util.tab_vc2)
    return varchar2

This would work well when calling from PL/SQL ex:

oos_util_string.multi_replace('hello world', oos_util.tab_vc2('hello','world'), oos_util.tab_vc2('goodnight', 'moon');

The only negative thing is that you can't call it from SQL as it's not a recognized type.

Let me know what you think.

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